All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	kvm@vger.kernel.org, kernel-team@android.com,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 22/23] KVM: arm64: Add a rVIC/rVID in-kernel implementation
Date: Sat, 05 Sep 2020 14:16:06 +0100	[thread overview]
Message-ID: <874kocmkix.wl-maz@kernel.org> (raw)
In-Reply-To: <20200904170036.00003bda@Huawei.com>

On Fri, 04 Sep 2020 17:00:36 +0100,
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> 
> On Thu, 3 Sep 2020 16:26:09 +0100
> Marc Zyngier <maz@kernel.org> wrote:

[...]

> > +static int rvic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
> > +{
> > +	struct rvic_vm_data *data;
> > +	struct kvm_vcpu *vcpu;
> > +	u32 __user *uaddr, val;
> > +	u16 trusted, total;
> > +	int i, ret = -ENXIO;
> > +
> > +	mutex_lock(&dev->kvm->lock);
> > +
> > +	switch (attr->group) {
> > +	case KVM_DEV_ARM_RVIC_GRP_NR_IRQS:
> > +		if (attr->attr)
> > +			break;
> > +
> > +		if (dev->kvm->arch.irqchip_data) {
> > +			ret = -EBUSY;
> > +			break;
> > +		}
> > +
> > +		uaddr = (u32 __user *)(uintptr_t)attr->addr;
> > +		if (get_user(val, uaddr)) {
> > +			ret = -EFAULT;
> > +			break;
> > +		}
> > +
> > +		trusted = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TRUSTED_MASK, val);
> > +		total   = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TOTAL_MASK, val);
> > +		if (total < trusted || trusted < 32 || total < 64 ||
> > +		    trusted % 32 || total % 32 || total > 2048) {
> 
> As I read the spec, we need at least 32 untrusted. (R0058) 
> This condition seems to allow that if trusted = 64 and untrusted = 0

Well spotted. I think the following would capture the constraints
correctly:

		if (total <= trusted || trusted < 32 || total < 64 ||
		    trusted % 32 || total % 32 || total > 2048) {

On the other hand, I wonder if this code would gain from being
directly written in terms of trusted/untrusted, rather than
trusted/total. It could make the reading against the spec easier.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	kvm@vger.kernel.org, Suzuki K Poulose <suzuki.poulose@arm.com>,
	Christoffer Dall <Christoffer.Dall@arm.com>,
	James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	kernel-team@android.com, kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 22/23] KVM: arm64: Add a rVIC/rVID in-kernel implementation
Date: Sat, 05 Sep 2020 14:16:06 +0100	[thread overview]
Message-ID: <874kocmkix.wl-maz@kernel.org> (raw)
In-Reply-To: <20200904170036.00003bda@Huawei.com>

On Fri, 04 Sep 2020 17:00:36 +0100,
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> 
> On Thu, 3 Sep 2020 16:26:09 +0100
> Marc Zyngier <maz@kernel.org> wrote:

[...]

> > +static int rvic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
> > +{
> > +	struct rvic_vm_data *data;
> > +	struct kvm_vcpu *vcpu;
> > +	u32 __user *uaddr, val;
> > +	u16 trusted, total;
> > +	int i, ret = -ENXIO;
> > +
> > +	mutex_lock(&dev->kvm->lock);
> > +
> > +	switch (attr->group) {
> > +	case KVM_DEV_ARM_RVIC_GRP_NR_IRQS:
> > +		if (attr->attr)
> > +			break;
> > +
> > +		if (dev->kvm->arch.irqchip_data) {
> > +			ret = -EBUSY;
> > +			break;
> > +		}
> > +
> > +		uaddr = (u32 __user *)(uintptr_t)attr->addr;
> > +		if (get_user(val, uaddr)) {
> > +			ret = -EFAULT;
> > +			break;
> > +		}
> > +
> > +		trusted = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TRUSTED_MASK, val);
> > +		total   = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TOTAL_MASK, val);
> > +		if (total < trusted || trusted < 32 || total < 64 ||
> > +		    trusted % 32 || total % 32 || total > 2048) {
> 
> As I read the spec, we need at least 32 untrusted. (R0058) 
> This condition seems to allow that if trusted = 64 and untrusted = 0

Well spotted. I think the following would capture the constraints
correctly:

		if (total <= trusted || trusted < 32 || total < 64 ||
		    trusted % 32 || total % 32 || total > 2048) {

On the other hand, I wonder if this code would gain from being
directly written in terms of trusted/untrusted, rather than
trusted/total. It could make the reading against the spec easier.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Cc: <kvm@vger.kernel.org>, <kvmarm@lists.cs.columbia.edu>,
	<linux-arm-kernel@lists.infradead.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Christoffer Dall <Christoffer.Dall@arm.com>,
	James Morse <james.morse@arm.com>, <kernel-team@android.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>
Subject: Re: [PATCH 22/23] KVM: arm64: Add a rVIC/rVID in-kernel implementation
Date: Sat, 05 Sep 2020 14:16:06 +0100	[thread overview]
Message-ID: <874kocmkix.wl-maz@kernel.org> (raw)
In-Reply-To: <20200904170036.00003bda@Huawei.com>

On Fri, 04 Sep 2020 17:00:36 +0100,
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> 
> On Thu, 3 Sep 2020 16:26:09 +0100
> Marc Zyngier <maz@kernel.org> wrote:

[...]

> > +static int rvic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
> > +{
> > +	struct rvic_vm_data *data;
> > +	struct kvm_vcpu *vcpu;
> > +	u32 __user *uaddr, val;
> > +	u16 trusted, total;
> > +	int i, ret = -ENXIO;
> > +
> > +	mutex_lock(&dev->kvm->lock);
> > +
> > +	switch (attr->group) {
> > +	case KVM_DEV_ARM_RVIC_GRP_NR_IRQS:
> > +		if (attr->attr)
> > +			break;
> > +
> > +		if (dev->kvm->arch.irqchip_data) {
> > +			ret = -EBUSY;
> > +			break;
> > +		}
> > +
> > +		uaddr = (u32 __user *)(uintptr_t)attr->addr;
> > +		if (get_user(val, uaddr)) {
> > +			ret = -EFAULT;
> > +			break;
> > +		}
> > +
> > +		trusted = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TRUSTED_MASK, val);
> > +		total   = FIELD_GET(KVM_DEV_ARM_RVIC_GRP_NR_TOTAL_MASK, val);
> > +		if (total < trusted || trusted < 32 || total < 64 ||
> > +		    trusted % 32 || total % 32 || total > 2048) {
> 
> As I read the spec, we need at least 32 untrusted. (R0058) 
> This condition seems to allow that if trusted = 64 and untrusted = 0

Well spotted. I think the following would capture the constraints
correctly:

		if (total <= trusted || trusted < 32 || total < 64 ||
		    trusted % 32 || total % 32 || total > 2048) {

On the other hand, I wonder if this code would gain from being
directly written in terms of trusted/untrusted, rather than
trusted/total. It could make the reading against the spec easier.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

  reply	other threads:[~2020-09-05 13:16 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-03 15:25 [PATCH 00/23] KVM: arm64: rVIC/rVID PV interrupt controller Marc Zyngier
2020-09-03 15:25 ` Marc Zyngier
2020-09-03 15:25 ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 01/23] irqchip: Add Reduced Virtual Interrupt Controller driver Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 02/23] irqchip/rvic: Add support for untrusted interrupt allocation Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-04 13:40   ` Jonathan Cameron
2020-09-04 13:40     ` Jonathan Cameron
2020-09-04 13:40     ` Jonathan Cameron
2020-09-03 15:25 ` [PATCH 03/23] irqchip: Add Reduced Virtual Interrupt Distributor support Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-04 13:56   ` Jonathan Cameron
2020-09-04 13:56     ` Jonathan Cameron
2020-09-04 13:56     ` Jonathan Cameron
2020-09-03 15:25 ` [PATCH 04/23] irqchip/rvid: Add PCI MSI support Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-04 14:15   ` Jonathan Cameron
2020-09-04 14:15     ` Jonathan Cameron
2020-09-04 14:15     ` Jonathan Cameron
2020-09-05 13:08     ` Marc Zyngier
2020-09-05 13:08       ` Marc Zyngier
2020-09-05 13:08       ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 05/23] KVM: arm64: Move GIC model out of the distributor Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-04 14:37   ` Jonathan Cameron
2020-09-04 14:37     ` Jonathan Cameron
2020-09-04 14:37     ` Jonathan Cameron
2020-09-03 15:25 ` [PATCH 06/23] KVM: arm64: vgic-v3: Move early init to kvm_vgic_create() Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 07/23] KVM: arm64: Add irqchip callback structure to kvm_arch Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 08/23] KVM: arm64: Move kvm_vgic_destroy to kvm_irqchip_flow Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 09/23] KVM: arm64: Move kvm_vgic_vcpu_init() to irqchip_flow Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 10/23] KVM: arm64: Move kvm_vgic_vcpu_[un]blocking() " Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 11/23] KVM: arm64: Move kvm_vgic_vcpu_load/put() " Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25 ` [PATCH 12/23] KVM: arm64: Move kvm_vgic_vcpu_pending_irq() " Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-03 15:25   ` Marc Zyngier
2020-09-04 14:57   ` Jonathan Cameron
2020-09-04 14:57     ` Jonathan Cameron
2020-09-04 14:57     ` Jonathan Cameron
2020-09-03 15:26 ` [PATCH 13/23] KVM: arm64: Move vgic resource mapping on first run " Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 14/23] KVM: arm64: Move kvm_vgic_vcpu_{sync, flush}_hwstate() " Marc Zyngier
2020-09-03 15:26   ` [PATCH 14/23] KVM: arm64: Move kvm_vgic_vcpu_{sync,flush}_hwstate() " Marc Zyngier
2020-09-03 15:26   ` [PATCH 14/23] KVM: arm64: Move kvm_vgic_vcpu_{sync, flush}_hwstate() " Marc Zyngier
2020-09-03 15:26 ` [PATCH 15/23] KVM: arm64: nVHE: Only save/restore GICv3 state if modeling a GIC Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 16/23] KVM: arm64: Move interrupt injection to irqchip_flow Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 17/23] KVM: arm64: Move mapping of HW interrupts into irqchip_flow Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 18/23] KVM: arm64: Move set_owner " Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 19/23] KVM: arm64: Turn vgic_initialized into irqchip_finalized Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 20/23] KVM: arm64: Move irqfd routing to irqchip_flow Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 21/23] KVM: arm64: Tighten msis_require_devid reporting Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26 ` [PATCH 22/23] KVM: arm64: Add a rVIC/rVID in-kernel implementation Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-04 16:00   ` Jonathan Cameron
2020-09-04 16:00     ` Jonathan Cameron
2020-09-04 16:00     ` Jonathan Cameron
2020-09-05 13:16     ` Marc Zyngier [this message]
2020-09-05 13:16       ` Marc Zyngier
2020-09-05 13:16       ` Marc Zyngier
2020-09-29 15:13   ` Lorenzo Pieralisi
2020-09-29 15:13     ` Lorenzo Pieralisi
2020-09-29 15:13     ` Lorenzo Pieralisi
2020-09-29 15:27     ` Marc Zyngier
2020-09-29 15:27       ` Marc Zyngier
2020-09-29 15:27       ` Marc Zyngier
2020-09-29 16:09       ` Lorenzo Pieralisi
2020-09-29 16:09         ` Lorenzo Pieralisi
2020-09-29 16:09         ` Lorenzo Pieralisi
2020-09-03 15:26 ` [PATCH 23/23] KVM: arm64: Add debugfs files for the rVIC/rVID implementation Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier
2020-09-03 15:26   ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874kocmkix.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=kernel-team@android.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=lorenzo.pieralisi@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.