[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 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. > > >> + { >> + rcu_unlock_domain(d); >> + return -EOPNOTSUPP; >> + } >> + rcu_unlock_domain(d); >> + >> switch ( map.type ) >> { >> case MAP_PIRQ_TYPE_MSI_SEG: >> @@ -341,11 +352,22 @@ ret_t do_physdev_op(int cmd, >> XEN_GUEST_HANDLE_PARAM(void) arg) >> >> case PHYSDEVOP_unmap_pirq: { >> struct physdev_unmap_pirq unmap; >> + struct domain *d; >> >> ret = -EFAULT; >> if ( copy_from_guest(&unmap, arg, 1) != 0 ) >> break; >> >> + d = rcu_lock_domain_by_any_id(unmap.domid); >> + if ( d == NULL ) >> + return -ESRCH; >> + if ( !is_pv_domain(d) && !has_pirq(d) ) > > same here > > > Other than that, everything looks fine to me > > >> + { >> + rcu_unlock_domain(d); >> + return -EOPNOTSUPP; >> + } >> + rcu_unlock_domain(d); >> + >> ret = physdev_unmap_pirq(unmap.domid, unmap.pirq); >> break; >> } >> -- >> 2.34.1 >> -- Best regards, Jiqian Chen.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |