[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v14 2/5] x86/pvh: Allow (un)map_pirq when dom0 is PVH
On 2024/9/3 17:25, Jan Beulich wrote: > On 03.09.2024 09:58, Chen, Jiqian wrote: >> On 2024/9/3 15:42, Jan Beulich wrote: >>> On 03.09.2024 09:04, Jiqian Chen wrote: >>>> When dom0 is PVH type and passthrough a device to HVM domU, Qemu code >>>> xen_pt_realize->xc_physdev_map_pirq and libxl code pci_add_dm_done-> >>>> xc_physdev_map_pirq map a pirq for passthrough devices. >>>> In xc_physdev_map_pirq call stack, function hvm_physdev_op has a check >>>> has_pirq(currd), but currd is PVH dom0, PVH has no X86_EMU_USE_PIRQ flag, >>>> so it fails, PHYSDEVOP_map_pirq is not allowed for PVH dom0 in current >>>> codes. >>>> >>>> But it is fine to map interrupts through pirq to a HVM domain whose >>>> XENFEAT_hvm_pirqs is not enabled. Because pirq field is used as a way to >>>> reference interrupts and it is just the way for the device model to >>>> identify which interrupt should be mapped to which domain, however >>>> has_pirq() is just to check if HVM domains route interrupts from >>>> devices(emulated or passthrough) through event channel, so, the has_pirq() >>>> check should not be applied to the PHYSDEVOP_map_pirq issued by dom0. >>>> >>>> So, allow PHYSDEVOP_map_pirq when dom0 is PVH and also allow >>>> PHYSDEVOP_unmap_pirq for the removal device path to unmap pirq. Then the >>>> interrupt of a passthrough device can be successfully mapped to pirq for >>>> domU. >>> >>> As before: When you talk about just Dom0, ... >>> >>>> --- a/xen/arch/x86/hvm/hypercall.c >>>> +++ b/xen/arch/x86/hvm/hypercall.c >>>> @@ -73,6 +73,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: >>> >>> ... that ought to match the code. IOW you've again lost why it is okay(ish) >>> (or even necessary) to also permit the op for non-Dom0 (e.g. a PVH stubdom). >>> Similarly imo Dom0 using this on itself wants discussing. >> Do you mean I need to talk about why permit this op for all HVM > > You don't need to invent reasons, but it needs making clear that wider than > necessary (for your purpose) exposure is at least not going to be a problem. > >> and where can prevent PVH domain calling this for self-mapping, not only >> dom0? > > Aiui use on itself is limited to Dom0, so only that would need clarifying > (along the lines of the above, i.e. that/why it is not a problem). For > has_pirq() domains use on oneself was already permitted before. Changed commit message to below. Please check and that will be great helpful if you would show me how to modify it. { x86/pvh: Allow (un)map_pirq when dom0 is PVH Problem: when dom0 is PVH type and passthrough a device to HVM domU, Qemu code xen_pt_realize->xc_physdev_map_pirq and libxl code pci_add_dm_done-> xc_physdev_map_pirq map a pirq for passthrough devices. In xc_physdev_map_pirq call stack, function hvm_physdev_op has a check has_pirq(currd), but currd is PVH dom0, PVH has no X86_EMU_USE_PIRQ flag, so it fails, PHYSDEVOP_map_pirq is not allowed for PVH dom0 in current codes. To solve above problem, need to remove the chack has_pirq() for that situation(PHYSDEVOP_map_pirq is issued by dom0 for domUs). But without adding other restrictions, PHYSDEVOP_map_pirq will be allowed wider than what the problem need. So, clarify below: For HVM domUs whose XENFEAT_hvm_pirqs is not enabled,it is fine to map interrupts through pirq for them. Because pirq field is used as a way to reference interrupts and it is just the way for the device model to identify which interrupt should be mapped to which domain, however has_pirq() is just to check if HVM domains route interrupts from devices(emulated or passthrough) through event channel, so, remove has_pirq() check has no impact on HVM domUs. For PVH domUs that performs such an operation will fail at the check xsm_map_dedomain_pirq() in physdev_map-nirq(). For PVH dom0, it uses vpci and doesn't use event channel, as above talks, it also has no impact. } > > Jan -- Best regards, Jiqian Chen.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |