| Message ID | 20230312075455.450187-4-ray.huang@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce VirtIO GPU and Passthrough GPU support on Xen PVH dom0 | expand |
On 12.03.2023 08:54, Huang Rui wrote: > From: Chen Jiqian <Jiqian.Chen@amd.com> > > PVH is also hvm type domain, but PVH hasn't X86_EMU_USE_PIRQ > flag. So, when dom0 is PVH and call PHYSDEVOP_map_pirq, it > will fail at check has_pirq(); > > Signed-off-by: Chen Jiqian <Jiqian.Chen@amd.com> > Signed-off-by: Huang Rui <ray.huang@amd.com> Please see b96b50004804 ("x86: remove XENFEAT_hvm_pirqs for PVHv2 guests"), which clearly says that these sub-ops shouldn't be used by PVH domains. Plus if you're after just one sub-op (assuming that indeed needs making available for a yet to be supplied reason), why ... > --- a/xen/arch/x86/hvm/hypercall.c > +++ b/xen/arch/x86/hvm/hypercall.c > @@ -89,8 +89,6 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > case PHYSDEVOP_eoi: > case PHYSDEVOP_irq_status_query: > case PHYSDEVOP_get_free_pirq: > - if ( !has_pirq(currd) ) > - return -ENOSYS; > break; ... do you enable several more by simply dropping code altogether? Jan
On Sun, Mar 12, 2023 at 03:54:52PM +0800, Huang Rui wrote: > From: Chen Jiqian <Jiqian.Chen@amd.com> > > PVH is also hvm type domain, but PVH hasn't X86_EMU_USE_PIRQ > flag. So, when dom0 is PVH and call PHYSDEVOP_map_pirq, it > will fail at check has_pirq(); > > Signed-off-by: Chen Jiqian <Jiqian.Chen@amd.com> > Signed-off-by: Huang Rui <ray.huang@amd.com> > --- > xen/arch/x86/hvm/hypercall.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c > index 405d0a95af..16a2f5c0b3 100644 > --- a/xen/arch/x86/hvm/hypercall.c > +++ b/xen/arch/x86/hvm/hypercall.c > @@ -89,8 +89,6 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > case PHYSDEVOP_eoi: > case PHYSDEVOP_irq_status_query: > case PHYSDEVOP_get_free_pirq: > - if ( !has_pirq(currd) ) > - return -ENOSYS; Since I've taken a look at the Linux side of this, it seems like you need PHYSDEVOP_map_pirq and PHYSDEVOP_setup_gsi, but the later is not in this list because has never been available to HVM type guests. I would like to better understand the usage by PVH dom0 for GSI passthrough before deciding on what to do here. IIRC QEMU also uses PHYSDEVOP_{un,}map_pirq in order to allocate MSI(-X) interrupts. Thanks, Roger.
On Wed, 15 Mar 2023, Roger Pau Monné wrote: > On Sun, Mar 12, 2023 at 03:54:52PM +0800, Huang Rui wrote: > > From: Chen Jiqian <Jiqian.Chen@amd.com> > > > > PVH is also hvm type domain, but PVH hasn't X86_EMU_USE_PIRQ > > flag. So, when dom0 is PVH and call PHYSDEVOP_map_pirq, it > > will fail at check has_pirq(); > > > > Signed-off-by: Chen Jiqian <Jiqian.Chen@amd.com> > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > --- > > xen/arch/x86/hvm/hypercall.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c > > index 405d0a95af..16a2f5c0b3 100644 > > --- a/xen/arch/x86/hvm/hypercall.c > > +++ b/xen/arch/x86/hvm/hypercall.c > > @@ -89,8 +89,6 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > > case PHYSDEVOP_eoi: > > case PHYSDEVOP_irq_status_query: > > case PHYSDEVOP_get_free_pirq: > > - if ( !has_pirq(currd) ) > > - return -ENOSYS; > > Since I've taken a look at the Linux side of this, it seems like you > need PHYSDEVOP_map_pirq and PHYSDEVOP_setup_gsi, but the later is not > in this list because has never been available to HVM type guests. > > I would like to better understand the usage by PVH dom0 for GSI > passthrough before deciding on what to do here. IIRC QEMU also uses > PHYSDEVOP_{un,}map_pirq in order to allocate MSI(-X) interrupts. I'll let Ray reply here, but I think you are right: HYSDEVOP_{un,}map_pirq are needed so that QEMU can run in PVH Dom0 to support HVM guests.
On Wed, Mar 15, 2023 at 11:57:45PM +0800, Roger Pau Monné wrote: > On Sun, Mar 12, 2023 at 03:54:52PM +0800, Huang Rui wrote: > > From: Chen Jiqian <Jiqian.Chen@amd.com> > > > > PVH is also hvm type domain, but PVH hasn't X86_EMU_USE_PIRQ > > flag. So, when dom0 is PVH and call PHYSDEVOP_map_pirq, it > > will fail at check has_pirq(); > > > > Signed-off-by: Chen Jiqian <Jiqian.Chen@amd.com> > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > --- > > xen/arch/x86/hvm/hypercall.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c > > index 405d0a95af..16a2f5c0b3 100644 > > --- a/xen/arch/x86/hvm/hypercall.c > > +++ b/xen/arch/x86/hvm/hypercall.c > > @@ -89,8 +89,6 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > > case PHYSDEVOP_eoi: > > case PHYSDEVOP_irq_status_query: > > case PHYSDEVOP_get_free_pirq: > > - if ( !has_pirq(currd) ) > > - return -ENOSYS; > > Since I've taken a look at the Linux side of this, it seems like you > need PHYSDEVOP_map_pirq and PHYSDEVOP_setup_gsi, but the later is not > in this list because has never been available to HVM type guests. Do you mean HVM guest only support MSI(-X)? > > I would like to better understand the usage by PVH dom0 for GSI > passthrough before deciding on what to do here. IIRC QEMU also uses > PHYSDEVOP_{un,}map_pirq in order to allocate MSI(-X) interrupts. > The MSI(-X) interrupt doesn't work even on the passthrough device at domU even the dom0 is PV domain. It seems a common problem, I remember Christian encountered the similar issue as well. So we fallback to use the GSI interrupt instead. Thanks, Ray
On 21.03.2023 11:09, Huang Rui wrote: > On Wed, Mar 15, 2023 at 11:57:45PM +0800, Roger Pau Monné wrote: >> On Sun, Mar 12, 2023 at 03:54:52PM +0800, Huang Rui wrote: >>> From: Chen Jiqian <Jiqian.Chen@amd.com> >>> >>> PVH is also hvm type domain, but PVH hasn't X86_EMU_USE_PIRQ >>> flag. So, when dom0 is PVH and call PHYSDEVOP_map_pirq, it >>> will fail at check has_pirq(); >>> >>> Signed-off-by: Chen Jiqian <Jiqian.Chen@amd.com> >>> Signed-off-by: Huang Rui <ray.huang@amd.com> >>> --- >>> xen/arch/x86/hvm/hypercall.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c >>> index 405d0a95af..16a2f5c0b3 100644 >>> --- a/xen/arch/x86/hvm/hypercall.c >>> +++ b/xen/arch/x86/hvm/hypercall.c >>> @@ -89,8 +89,6 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) >>> case PHYSDEVOP_eoi: >>> case PHYSDEVOP_irq_status_query: >>> case PHYSDEVOP_get_free_pirq: >>> - if ( !has_pirq(currd) ) >>> - return -ENOSYS; >> >> Since I've taken a look at the Linux side of this, it seems like you >> need PHYSDEVOP_map_pirq and PHYSDEVOP_setup_gsi, but the later is not >> in this list because has never been available to HVM type guests. > > Do you mean HVM guest only support MSI(-X)? I don't think that was meant. Instead, as per discussion elsewhere, we may need to make PHYSDEVOP_setup_gsi available to PVH Dom0. (DomU-s wouldn't be allowed to use this sub-op, so the statement Roger made simply doesn't apply to "HVM guest".) >> I would like to better understand the usage by PVH dom0 for GSI >> passthrough before deciding on what to do here. IIRC QEMU also uses >> PHYSDEVOP_{un,}map_pirq in order to allocate MSI(-X) interrupts. >> > > The MSI(-X) interrupt doesn't work even on the passthrough device at domU > even the dom0 is PV domain. It seems a common problem, I remember Christian > encountered the similar issue as well. So we fallback to use the GSI > interrupt instead. Looks like this wants figuring out properly as well then. MSI(-X) generally works for pass-through devices, from all I know. Jan
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c index 405d0a95af..16a2f5c0b3 100644 --- a/xen/arch/x86/hvm/hypercall.c +++ b/xen/arch/x86/hvm/hypercall.c @@ -89,8 +89,6 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) case PHYSDEVOP_eoi: case PHYSDEVOP_irq_status_query: case PHYSDEVOP_get_free_pirq: - if ( !has_pirq(currd) ) - return -ENOSYS; break; case PHYSDEVOP_pci_mmcfg_reserved: