Re: [XEN PATCH v13 2/6] x86/pvh: Allow (un)map_pirq when dom0 is PVH

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

On 2024/8/20 15:07, Jan Beulich wrote:
> On 20.08.2024 08:12, Chen, Jiqian wrote:
>> On 2024/8/19 17:08, Jan Beulich wrote:
>>> On 16.08.2024 13:08, 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
>>>> iPHYSDEVOP_unmap_pirq for the removal device path to unmap pirq.
>>>> So that the interrupt of a passthrough device can be successfully
>>>> mapped to pirq for domU with a notion of PIRQ when dom0 is PVH.
>>>>
>>>> To exposing the functionality to wider than (presently) necessary
>>>> audience(like PVH domU), so it doesn't add any futher restrictions.
>>>
>>> The code change is fine, but I'm struggling with this sentence. I can't
>>> really derive what you're trying to say.
>> Ah, I wanted to explain why this path not add any further restrictions, then 
>> used your comments of last version.
>> How do I need to change this explanation?
> 
> I think you want to take Roger's earlier comments (when he requested
> the relaxation) as basis to re-write (combine) both of the latter two
> paragraphs above (or maybe even all three of them). It's odd to first
> talk about Dom0, as if the operations were to be exposed just there,
> and only then add DomU-s.

I tried to understand and summarize Roger's previous comments and changed 
commit message to the following. Do you think it is fine?

x86/pvh: Allow (un)map_pirq when dom0 is PVH

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.

And the PVH domU which use vpci trying to issue a map_pirq will fail at the
xsm_map_domain_pirq() check in physdev_map_pirq() .

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.

> 
>>>> And there already are some senarios for domains without
>>>> X86_EMU_USE_PIRQ to use these functions.
>>>
>>> Are there? If so, pointing out an example may help.
>> If I understand correctly, Roger mentioned that PIRQs is disable by default 
>> for HVM guest("hvm_pirq=0") and passthrough device to guest.
>> In this scene, guest doesn't have PIRQs, but it still needs this hypercall.
> 
> In which case please say so in order to be concrete, not vague.
> 
> Jan

-- 
Best regards,
Jiqian Chen.