On 2024/6/18 16:38, Jan Beulich wrote: > On 18.06.2024 08:49, Chen, Jiqian wrote: >> On 2024/6/17 22:45, Jan Beulich wrote: >>> On 17.06.2024 11:00, 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 qemu code >>>> xen_pt_realize->xc_physdev_map_pirq and libxl code >>>> 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. >>> >>> Why "failed path"? Isn't unmapping also part of normal device removal >>> from a guest? >> Yes, both. I will change to also "allow PHYSDEVOP_unmap_pirq for the device >> removal path to unmap pirq". >> >>> >>>> And >>>> add a new check to prevent self map when subject domain has no >>>> PIRQ flag. >>> >>> You still talk of only self mapping, and the code also still does only >>> that. As pointed out before: Why would you allow mapping into a PVH >>> DomU? IOW what purpose do the "d == currd" checks have? >> The checking I added has two purpose, first is I need to allow this case: >> Dom0(without PIRQ) + DomU(with PIRQ), because the original code just do >> (!has_pirq(currd)) will cause map_pirq fail in this case. >> Second I need to disallow self-mapping: >> DomU(without PIRQ) do map_pirq, the "d==currd" means the currd is the >> subject domain itself. >> >> Emmm, I think I know what's your concern. >> Do you mean I need to >> " Prevent map_pirq when currd has no X86_EMU_USE_PIRQ flag " >> instead of >> " Prevent self-map when currd has no X86_EMU_USE_PIRQ flag ", > > No. What I mean is that I continue to fail to see why you mention "currd". > IOW it would be more like "prevent mapping when the subject domain has no > X86_EMU_USE_PIRQ" (which, as a specific sub-case, includes self-mapping > if the caller specifies DOMID_SELF for the subject domain). Oh, I see, not only to prevent self-mapping, but if the subject domain has no PIRQs, we should reject, self-mapping is just the one sub case.
> >> so I need to remove "d==currd", right? > > Removing this check is what I'm after, yes. Yet that's not in sync with > either of the two quoted sentences above. > >>>> So that domU with PIRQ flag can success to map pirq for >>>> passthrough devices even dom0 has no PIRQ flag. >>> >>> There's still a description problem here. Much like the first sentence, >>> this last one also says that the guest would itself map the pIRQ. In >>> which case there would still not be any reason to expose the sub- >>> functions to Dom0. >> If change to " So that the interrupt of a passthrough device can success to >> be mapped to pirq for domU with PIRQ flag when dom0 is PVH.", >> Is it OK? > > Kind of, yes. "can be successfully mapped" is one of the various possibilities > of making this read a little more smoothly. OK. > > Jan -- Best regards, Jiqian Chen.
