[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC XEN PATCH v5 2/5] x86/pvh: Allow (un)map_pirq when dom0 is PVH
On Fri, 23 Feb 2024, Chen, Jiqian wrote: > On 2024/2/23 08:40, Stefano Stabellini wrote: > > On Fri, 12 Jan 2024, Jiqian Chen wrote: > >> If run Xen with PVH dom0 and hvm domU, hvm will map a pirq for > >> a passthrough device by using gsi, see > >> xen_pt_realize->xc_physdev_map_pirq and > >> pci_add_dm_done->xc_physdev_map_pirq. Then xc_physdev_map_pirq > >> will call into Xen, but in hvm_physdev_op, PHYSDEVOP_map_pirq > >> is not allowed because currd is PVH dom0 and PVH has no > >> X86_EMU_USE_PIRQ flag, it will fail at has_pirq check. > >> > >> So, allow PHYSDEVOP_map_pirq when dom0 is PVH and also allow > >> PHYSDEVOP_unmap_pirq for the failed path to unmap pirq. And > >> add a new check to prevent self map when caller has no PIRQ > >> flag. > >> > >> Co-developed-by: Huang Rui <ray.huang@xxxxxxx> > >> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx> > >> --- > >> xen/arch/x86/hvm/hypercall.c | 2 ++ > >> xen/arch/x86/physdev.c | 22 ++++++++++++++++++++++ > >> 2 files changed, 24 insertions(+) > >> > >> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c > >> index 6ad5b4d5f11f..493998b42ec5 100644 > >> --- a/xen/arch/x86/hvm/hypercall.c > >> +++ b/xen/arch/x86/hvm/hypercall.c > >> @@ -74,6 +74,8 @@ long hvm_physdev_op(int cmd, > >> XEN_GUEST_HANDLE_PARAM(void) arg) > >> { > >> case PHYSDEVOP_map_pirq: > >> case PHYSDEVOP_unmap_pirq: > >> + break; > >> + > >> case PHYSDEVOP_eoi: > >> case PHYSDEVOP_irq_status_query: > >> case PHYSDEVOP_get_free_pirq: > >> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c > >> index 47c4da0af7e1..7f2422c2a483 100644 > >> --- a/xen/arch/x86/physdev.c > >> +++ b/xen/arch/x86/physdev.c > >> @@ -303,11 +303,22 @@ ret_t do_physdev_op(int cmd, > >> XEN_GUEST_HANDLE_PARAM(void) arg) > >> case PHYSDEVOP_map_pirq: { > >> physdev_map_pirq_t map; > >> struct msi_info msi; > >> + struct domain *d; > >> > >> ret = -EFAULT; > >> if ( copy_from_guest(&map, arg, 1) != 0 ) > >> break; > >> > >> + d = rcu_lock_domain_by_any_id(map.domid); > >> + if ( d == NULL ) > >> + return -ESRCH; > >> + if ( !is_pv_domain(d) && !has_pirq(d) ) > > > > I think this could just be: > > > > if ( !has_pirq(d) ) > > > > Right? > No. In the beginning, I only set this condition here, but it destroyed PV > dom0. > Because PV has no pirq flag too, it can match this condition and return > -EOPNOTSUPP, PHYSDEVOP_map_pirq will fail. Yes I get it now. Thanks for the explanation. I think "has_pirq" is misnamed and should probably be hvm_has_pirq or something like that. I am not asking to fix it, but maybe an in-code comment here would help, like: /* if it is an HVM guest, check if it has PIRQs */ in any case: Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |