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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.09.2024 06:01, Chen, Jiqian wrote:
> 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?

What are we talking about here? The patch was committed over a month
ago?

Jan

> 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
>