* [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking
@ 2019-03-22 5:23 Leo Yan
2019-03-22 5:29 ` Leo Yan
2019-04-10 12:42 ` Jean-Philippe Brucker
0 siblings, 2 replies; 6+ messages in thread
From: Leo Yan @ 2019-03-22 5:23 UTC (permalink / raw)
To: kvm, kvmarm, Will Deacon, Marc Zyngier, Jean-Philippe Brucker,
Eric Auger, Robin Murphy
Cc: Leo Yan
If MSI doesn't support per-vector masking capability and
PCI_MSI_FLAGS_MASKBIT isn't set in message control field, the function
vfio_pci_msi_vector_write() will directly bail out for this case and
every vector's 'virt_state' keeps setting bit VFIO_PCI_MSI_STATE_MASKED.
This results in the state maintained in 'virt_state' cannot really
reflect the MSI hardware state; finally it will mislead the function
vfio_pci_update_msi_entry() to skip IRQ forwarding with below flow:
vfio_pci_update_msi_entry() {
[...]
if (msi_is_masked(entry->virt_state) == msi_is_masked(entry->phys_state))
return 0; ==> skip IRQ forwarding
[...]
}
To fix this issue, when detect PCI_MSI_FLAGS_MASKBIT is not set in the
message control field, this patch simply clears bit
VFIO_PCI_MSI_STATE_MASKED for all vectors 'virt_state'; at the end
vfio_pci_update_msi_entry() can forward MSI IRQ successfully.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
vfio/pci.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/vfio/pci.c b/vfio/pci.c
index ba971eb..4fd24ac 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -363,8 +363,18 @@ static int vfio_pci_msi_vector_write(struct kvm *kvm, struct vfio_device *vdev,
struct vfio_pci_device *pdev = &vdev->pci;
struct msi_cap_64 *msi_cap_64 = PCI_CAP(&pdev->hdr, pdev->msi.pos);
- if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT))
+ if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT)) {
+ /*
+ * If MSI doesn't support per-vector masking capability,
+ * simply unmask for all vectors.
+ */
+ for (i = 0; i < pdev->msi.nr_entries; i++) {
+ entry = &pdev->msi.entries[i];
+ msi_set_masked(entry->virt_state, false);
+ }
+
return 0;
+ }
if (msi_cap_64->ctrl & PCI_MSI_FLAGS_64BIT)
mask_pos = PCI_MSI_MASK_64;
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking
2019-03-22 5:23 [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking Leo Yan
@ 2019-03-22 5:29 ` Leo Yan
2019-04-08 1:34 ` Leo Yan
2019-04-10 12:42 ` Jean-Philippe Brucker
1 sibling, 1 reply; 6+ messages in thread
From: Leo Yan @ 2019-03-22 5:29 UTC (permalink / raw)
To: kvm, kvmarm, Will Deacon, Marc Zyngier, Jean-Philippe Brucker,
Eric Auger, Robin Murphy
On Fri, Mar 22, 2019 at 01:23:08PM +0800, Leo Yan wrote:
> If MSI doesn't support per-vector masking capability and
> PCI_MSI_FLAGS_MASKBIT isn't set in message control field, the function
> vfio_pci_msi_vector_write() will directly bail out for this case and
> every vector's 'virt_state' keeps setting bit VFIO_PCI_MSI_STATE_MASKED.
>
> This results in the state maintained in 'virt_state' cannot really
> reflect the MSI hardware state; finally it will mislead the function
> vfio_pci_update_msi_entry() to skip IRQ forwarding with below flow:
>
> vfio_pci_update_msi_entry() {
>
> [...]
>
> if (msi_is_masked(entry->virt_state) == msi_is_masked(entry->phys_state))
> return 0; ==> skip IRQ forwarding
>
> [...]
> }
>
> To fix this issue, when detect PCI_MSI_FLAGS_MASKBIT is not set in the
> message control field, this patch simply clears bit
> VFIO_PCI_MSI_STATE_MASKED for all vectors 'virt_state'; at the end
> vfio_pci_update_msi_entry() can forward MSI IRQ successfully.
Just remind, this patch is for kvmtool but not for kernel. Sorry I
forget to add it in subject.
Thanks,
Leo Yan
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> vfio/pci.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/vfio/pci.c b/vfio/pci.c
> index ba971eb..4fd24ac 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -363,8 +363,18 @@ static int vfio_pci_msi_vector_write(struct kvm *kvm, struct vfio_device *vdev,
> struct vfio_pci_device *pdev = &vdev->pci;
> struct msi_cap_64 *msi_cap_64 = PCI_CAP(&pdev->hdr, pdev->msi.pos);
>
> - if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT))
> + if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT)) {
> + /*
> + * If MSI doesn't support per-vector masking capability,
> + * simply unmask for all vectors.
> + */
> + for (i = 0; i < pdev->msi.nr_entries; i++) {
> + entry = &pdev->msi.entries[i];
> + msi_set_masked(entry->virt_state, false);
> + }
> +
> return 0;
> + }
>
> if (msi_cap_64->ctrl & PCI_MSI_FLAGS_64BIT)
> mask_pos = PCI_MSI_MASK_64;
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking
@ 2019-04-08 1:34 ` Leo Yan
0 siblings, 0 replies; 6+ messages in thread
From: Leo Yan @ 2019-04-08 1:34 UTC (permalink / raw)
To: kvm, kvmarm, Will Deacon, Marc Zyngier, Jean-Philippe Brucker,
Eric Auger, Robin Murphy
Hi Jean-Philippe,
On Fri, Mar 22, 2019 at 01:29:24PM +0800, Leo Yan wrote:
> On Fri, Mar 22, 2019 at 01:23:08PM +0800, Leo Yan wrote:
> > If MSI doesn't support per-vector masking capability and
> > PCI_MSI_FLAGS_MASKBIT isn't set in message control field, the function
> > vfio_pci_msi_vector_write() will directly bail out for this case and
> > every vector's 'virt_state' keeps setting bit VFIO_PCI_MSI_STATE_MASKED.
> >
> > This results in the state maintained in 'virt_state' cannot really
> > reflect the MSI hardware state; finally it will mislead the function
> > vfio_pci_update_msi_entry() to skip IRQ forwarding with below flow:
> >
> > vfio_pci_update_msi_entry() {
> >
> > [...]
> >
> > if (msi_is_masked(entry->virt_state) == msi_is_masked(entry->phys_state))
> > return 0; ==> skip IRQ forwarding
> >
> > [...]
> > }
> >
> > To fix this issue, when detect PCI_MSI_FLAGS_MASKBIT is not set in the
> > message control field, this patch simply clears bit
> > VFIO_PCI_MSI_STATE_MASKED for all vectors 'virt_state'; at the end
> > vfio_pci_update_msi_entry() can forward MSI IRQ successfully.
>
> Just remind, this patch is for kvmtool but not for kernel. Sorry I
> forget to add it in subject.
Gentle ping. Not sure if this patch is reasonable, could you help to
give a review?
[...]
Thanks,
Leo Yan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking
@ 2019-04-08 1:34 ` Leo Yan
0 siblings, 0 replies; 6+ messages in thread
From: Leo Yan @ 2019-04-08 1:34 UTC (permalink / raw)
To: kvm, kvmarm, Will Deacon, Marc Zyngier, Jean-Philippe Brucker,
Eric Auger, Robin Murphy
Hi Jean-Philippe,
On Fri, Mar 22, 2019 at 01:29:24PM +0800, Leo Yan wrote:
> On Fri, Mar 22, 2019 at 01:23:08PM +0800, Leo Yan wrote:
> > If MSI doesn't support per-vector masking capability and
> > PCI_MSI_FLAGS_MASKBIT isn't set in message control field, the function
> > vfio_pci_msi_vector_write() will directly bail out for this case and
> > every vector's 'virt_state' keeps setting bit VFIO_PCI_MSI_STATE_MASKED.
> >
> > This results in the state maintained in 'virt_state' cannot really
> > reflect the MSI hardware state; finally it will mislead the function
> > vfio_pci_update_msi_entry() to skip IRQ forwarding with below flow:
> >
> > vfio_pci_update_msi_entry() {
> >
> > [...]
> >
> > if (msi_is_masked(entry->virt_state) == msi_is_masked(entry->phys_state))
> > return 0; ==> skip IRQ forwarding
> >
> > [...]
> > }
> >
> > To fix this issue, when detect PCI_MSI_FLAGS_MASKBIT is not set in the
> > message control field, this patch simply clears bit
> > VFIO_PCI_MSI_STATE_MASKED for all vectors 'virt_state'; at the end
> > vfio_pci_update_msi_entry() can forward MSI IRQ successfully.
>
> Just remind, this patch is for kvmtool but not for kernel. Sorry I
> forget to add it in subject.
Gentle ping. Not sure if this patch is reasonable, could you help to
give a review?
[...]
Thanks,
Leo Yan
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking
@ 2019-04-10 12:42 ` Jean-Philippe Brucker
0 siblings, 0 replies; 6+ messages in thread
From: Jean-Philippe Brucker @ 2019-04-10 12:42 UTC (permalink / raw)
To: Leo Yan, kvm, kvmarm, Will Deacon, Marc Zyngier, Eric Auger,
Robin Murphy
Hi Leo,
On 22/03/2019 05:23, Leo Yan wrote:
> If MSI doesn't support per-vector masking capability and
> PCI_MSI_FLAGS_MASKBIT isn't set in message control field, the function
> vfio_pci_msi_vector_write() will directly bail out for this case and
> every vector's 'virt_state' keeps setting bit VFIO_PCI_MSI_STATE_MASKED.
>
> This results in the state maintained in 'virt_state' cannot really
> reflect the MSI hardware state; finally it will mislead the function
> vfio_pci_update_msi_entry() to skip IRQ forwarding with below flow:
>
> vfio_pci_update_msi_entry() {
>
> [...]
>
> if (msi_is_masked(entry->virt_state) == msi_is_masked(entry->phys_state))
> return 0; ==> skip IRQ forwarding
>
> [...]
> }
>
> To fix this issue, when detect PCI_MSI_FLAGS_MASKBIT is not set in the
> message control field, this patch simply clears bit
> VFIO_PCI_MSI_STATE_MASKED for all vectors 'virt_state'; at the end
> vfio_pci_update_msi_entry() can forward MSI IRQ successfully.
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> vfio/pci.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/vfio/pci.c b/vfio/pci.c
> index ba971eb..4fd24ac 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -363,8 +363,18 @@ static int vfio_pci_msi_vector_write(struct kvm *kvm, struct vfio_device *vdev,
> struct vfio_pci_device *pdev = &vdev->pci;
> struct msi_cap_64 *msi_cap_64 = PCI_CAP(&pdev->hdr, pdev->msi.pos);
>
> - if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT))
> + if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT)) {
> + /*
> + * If MSI doesn't support per-vector masking capability,
> + * simply unmask for all vectors.
> + */
> + for (i = 0; i < pdev->msi.nr_entries; i++) {
> + entry = &pdev->msi.entries[i];
> + msi_set_masked(entry->virt_state, false);
> + }
> +
This seems like the wrong place for this fix.
vfio_pci_msi_vector_write() is called every time the guest pokes the MSI
capability, and checks whether the access was on the Mask Bits Register.
If the function doesn't support per-vector masking, then the register
isn't implemented and we shouldn't do anything here.
To fix the problem I think we need to set masked(virt_state) properly at
init time, instead of blindly setting it to true. In fact from the
guest's point of view, MSIs and MSI-X are unmasked (and disabled) at
boot, so you could always set masked(virt_state) to false, but it may be
safer to copy the actual state of the MSI's Mask Bits into virt_state,
since for MSI, it could be non zero. If per-vector masking isn't
supported, then the virt state should be false.
Thanks,
Jean
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking
@ 2019-04-10 12:42 ` Jean-Philippe Brucker
0 siblings, 0 replies; 6+ messages in thread
From: Jean-Philippe Brucker @ 2019-04-10 12:42 UTC (permalink / raw)
To: Leo Yan, kvm, kvmarm, Will Deacon, Marc Zyngier, Eric Auger,
Robin Murphy
Hi Leo,
On 22/03/2019 05:23, Leo Yan wrote:
> If MSI doesn't support per-vector masking capability and
> PCI_MSI_FLAGS_MASKBIT isn't set in message control field, the function
> vfio_pci_msi_vector_write() will directly bail out for this case and
> every vector's 'virt_state' keeps setting bit VFIO_PCI_MSI_STATE_MASKED.
>
> This results in the state maintained in 'virt_state' cannot really
> reflect the MSI hardware state; finally it will mislead the function
> vfio_pci_update_msi_entry() to skip IRQ forwarding with below flow:
>
> vfio_pci_update_msi_entry() {
>
> [...]
>
> if (msi_is_masked(entry->virt_state) == msi_is_masked(entry->phys_state))
> return 0; ==> skip IRQ forwarding
>
> [...]
> }
>
> To fix this issue, when detect PCI_MSI_FLAGS_MASKBIT is not set in the
> message control field, this patch simply clears bit
> VFIO_PCI_MSI_STATE_MASKED for all vectors 'virt_state'; at the end
> vfio_pci_update_msi_entry() can forward MSI IRQ successfully.
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> vfio/pci.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/vfio/pci.c b/vfio/pci.c
> index ba971eb..4fd24ac 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -363,8 +363,18 @@ static int vfio_pci_msi_vector_write(struct kvm *kvm, struct vfio_device *vdev,
> struct vfio_pci_device *pdev = &vdev->pci;
> struct msi_cap_64 *msi_cap_64 = PCI_CAP(&pdev->hdr, pdev->msi.pos);
>
> - if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT))
> + if (!(msi_cap_64->ctrl & PCI_MSI_FLAGS_MASKBIT)) {
> + /*
> + * If MSI doesn't support per-vector masking capability,
> + * simply unmask for all vectors.
> + */
> + for (i = 0; i < pdev->msi.nr_entries; i++) {
> + entry = &pdev->msi.entries[i];
> + msi_set_masked(entry->virt_state, false);
> + }
> +
This seems like the wrong place for this fix.
vfio_pci_msi_vector_write() is called every time the guest pokes the MSI
capability, and checks whether the access was on the Mask Bits Register.
If the function doesn't support per-vector masking, then the register
isn't implemented and we shouldn't do anything here.
To fix the problem I think we need to set masked(virt_state) properly at
init time, instead of blindly setting it to true. In fact from the
guest's point of view, MSIs and MSI-X are unmasked (and disabled) at
boot, so you could always set masked(virt_state) to false, but it may be
safer to copy the actual state of the MSI's Mask Bits into virt_state,
since for MSI, it could be non zero. If per-vector masking isn't
supported, then the virt state should be false.
Thanks,
Jean
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-10 12:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-22 5:23 [PATCH] vfio-pci: Fix MSI IRQ forwarding for without per-vector masking Leo Yan
2019-03-22 5:29 ` Leo Yan
2019-04-08 1:34 ` Leo Yan
2019-04-08 1:34 ` Leo Yan
2019-04-10 12:42 ` Jean-Philippe Brucker
2019-04-10 12:42 ` Jean-Philippe Brucker
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.