| Message ID | 20160802111058.31496-1-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/08/16 12:10, Andre Przywara wrote: > According to the KVM API documentation a successful MSI injection > should return a value > 0 on success. > Since we pass the return value of vgic_its_inject_msi() directly on > to upper layers and userland, we need to use the same semantics here. > Briefly tested with QEMU and kvmtool on GICv3 hardware and the model. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Applies on top of next-20160728. Let me know if I should use a > different base. > > Cheers, > Andre. > > virt/kvm/arm/vgic/vgic-its.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index 07411cf..3268250 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -468,6 +468,7 @@ static void vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its, > * Queries the KVM IO bus framework to get the ITS pointer from the given > * doorbell address. > * We then call vgic_its_trigger_msi() with the decoded data. > + * According to the KVM_SIGNAL_MSI API description returns > 0 on success. > */ > int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > { > @@ -493,7 +494,7 @@ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > vgic_its_trigger_msi(kvm, iodev->its, msi->devid, msi->data); > mutex_unlock(&iodev->its->its_lock); > > - return 0; > + return 1; Really? And what if: - ITS is not enabled? - or no ITTE? - or collection not mapped? - or LPIs not enabled? These are the tests performed by vgic_its_trigger_msi(), so maybe you show make this function return something useful, and propagate the value to userspace instead of blindly returning 1? Thanks, M.
On Tue, Aug 02, 2016 at 12:10:58PM +0100, Andre Przywara wrote: > According to the KVM API documentation a successful MSI injection > should return a value > 0 on success. > Since we pass the return value of vgic_its_inject_msi() directly on > to upper layers and userland, we need to use the same semantics here. > Briefly tested with QEMU and kvmtool on GICv3 hardware and the model. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Applies on top of next-20160728. Let me know if I should use a > different base. > > Cheers, > Andre. > > virt/kvm/arm/vgic/vgic-its.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index 07411cf..3268250 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -468,6 +468,7 @@ static void vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its, > * Queries the KVM IO bus framework to get the ITS pointer from the given > * doorbell address. > * We then call vgic_its_trigger_msi() with the decoded data. > + * According to the KVM_SIGNAL_MSI API description returns > 0 on success. > */ > int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > { > @@ -493,7 +494,7 @@ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > vgic_its_trigger_msi(kvm, iodev->its, msi->devid, msi->data); > mutex_unlock(&iodev->its->its_lock); > > - return 0; > + return 1; > } > > /* Requires the its_lock to be held. */ Do we have a clear understanding of what 'guest blocked the MSI' constitutes in the ARM world? Thanks, -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/08/16 13:23, Christoffer Dall wrote: > On Tue, Aug 02, 2016 at 12:10:58PM +0100, Andre Przywara wrote: >> According to the KVM API documentation a successful MSI injection >> should return a value > 0 on success. >> Since we pass the return value of vgic_its_inject_msi() directly on >> to upper layers and userland, we need to use the same semantics here. >> Briefly tested with QEMU and kvmtool on GICv3 hardware and the model. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> Applies on top of next-20160728. Let me know if I should use a >> different base. >> >> Cheers, >> Andre. >> >> virt/kvm/arm/vgic/vgic-its.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c >> index 07411cf..3268250 100644 >> --- a/virt/kvm/arm/vgic/vgic-its.c >> +++ b/virt/kvm/arm/vgic/vgic-its.c >> @@ -468,6 +468,7 @@ static void vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its, >> * Queries the KVM IO bus framework to get the ITS pointer from the given >> * doorbell address. >> * We then call vgic_its_trigger_msi() with the decoded data. >> + * According to the KVM_SIGNAL_MSI API description returns > 0 on success. >> */ >> int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >> { >> @@ -493,7 +494,7 @@ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >> vgic_its_trigger_msi(kvm, iodev->its, msi->devid, msi->data); >> mutex_unlock(&iodev->its->its_lock); >> >> - return 0; >> + return 1; >> } >> >> /* Requires the its_lock to be held. */ > > Do we have a clear understanding of what 'guest blocked the MSI' > constitutes in the ARM world? I would define it as "the guest has not configured the ITS so that the corresponding LPI could be successfully delivered". Which is any of the conditions where the interrupt cannot make it into the pending table, and is dropped on the floor. Thanks, M.
On Tue, Aug 02, 2016 at 01:28:44PM +0100, Marc Zyngier wrote: > On 02/08/16 13:23, Christoffer Dall wrote: > > On Tue, Aug 02, 2016 at 12:10:58PM +0100, Andre Przywara wrote: > >> According to the KVM API documentation a successful MSI injection > >> should return a value > 0 on success. > >> Since we pass the return value of vgic_its_inject_msi() directly on > >> to upper layers and userland, we need to use the same semantics here. > >> Briefly tested with QEMU and kvmtool on GICv3 hardware and the model. > >> > >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> > >> --- > >> Applies on top of next-20160728. Let me know if I should use a > >> different base. > >> > >> Cheers, > >> Andre. > >> > >> virt/kvm/arm/vgic/vgic-its.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > >> index 07411cf..3268250 100644 > >> --- a/virt/kvm/arm/vgic/vgic-its.c > >> +++ b/virt/kvm/arm/vgic/vgic-its.c > >> @@ -468,6 +468,7 @@ static void vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its, > >> * Queries the KVM IO bus framework to get the ITS pointer from the given > >> * doorbell address. > >> * We then call vgic_its_trigger_msi() with the decoded data. > >> + * According to the KVM_SIGNAL_MSI API description returns > 0 on success. > >> */ > >> int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > >> { > >> @@ -493,7 +494,7 @@ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > >> vgic_its_trigger_msi(kvm, iodev->its, msi->devid, msi->data); > >> mutex_unlock(&iodev->its->its_lock); > >> > >> - return 0; > >> + return 1; > >> } > >> > >> /* Requires the its_lock to be held. */ > > > > Do we have a clear understanding of what 'guest blocked the MSI' > > constitutes in the ARM world? > > I would define it as "the guest has not configured the ITS so that the > corresponding LPI could be successfully delivered". Which is any of the > conditions where the interrupt cannot make it into the pending table, > and is dropped on the floor. > sounds good to me. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c index 07411cf..3268250 100644 --- a/virt/kvm/arm/vgic/vgic-its.c +++ b/virt/kvm/arm/vgic/vgic-its.c @@ -468,6 +468,7 @@ static void vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its, * Queries the KVM IO bus framework to get the ITS pointer from the given * doorbell address. * We then call vgic_its_trigger_msi() with the decoded data. + * According to the KVM_SIGNAL_MSI API description returns > 0 on success. */ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) { @@ -493,7 +494,7 @@ int vgic_its_inject_msi(struct kvm *kvm, struct kvm_msi *msi) vgic_its_trigger_msi(kvm, iodev->its, msi->devid, msi->data); mutex_unlock(&iodev->its->its_lock); - return 0; + return 1; } /* Requires the its_lock to be held. */
According to the KVM API documentation a successful MSI injection should return a value > 0 on success. Since we pass the return value of vgic_its_inject_msi() directly on to upper layers and userland, we need to use the same semantics here. Briefly tested with QEMU and kvmtool on GICv3 hardware and the model. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- Applies on top of next-20160728. Let me know if I should use a different base. Cheers, Andre. virt/kvm/arm/vgic/vgic-its.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)