Re: [RFC XEN PATCH v3 2/3] x86/pvh: Add (un)map_pirq and setup_gsi for PVH dom0

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

On 2023/12/11 23:31, Roger Pau Monné wrote:
> On Mon, Dec 11, 2023 at 12:40:08AM +0800, 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 currd is dom0 no matter if
>> dom0 has X86_EMU_USE_PIRQ flag and also allow
>> PHYSDEVOP_unmap_pirq for the failed path to unmap pirq.
>>
>> What's more, in PVH dom0, the gsis don't get registered, but
>> the gsi of a passthrough device must be configured for it to
>> be able to be mapped into a hvm domU.
>> So, add PHYSDEVOP_setup_gsi for PVH dom0, because PVH dom0
>> will setup gsi during assigning a device to passthrough.
>>
>> Co-developed-by: Huang Rui <ray.huang@xxxxxxx>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
>> ---
>>  xen/arch/x86/hvm/hypercall.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
>> index 6ad5b4d5f1..621d789bd3 100644
>> --- a/xen/arch/x86/hvm/hypercall.c
>> +++ b/xen/arch/x86/hvm/hypercall.c
>> @@ -72,8 +72,11 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
>> arg)
>>  
>>      switch ( cmd )
>>      {
>> +    case PHYSDEVOP_setup_gsi:
> 
> I think given the new approach on the Linux side patches, where
> pciback will configure the interrupt, there's no need to expose
> setup_gsi anymore?
The latest patch(the second patch of v3 on kernel side) does setup_gsi and 
map_pirq for passthrough device in pciback, so we need this and below.

> 
>>      case PHYSDEVOP_map_pirq:
>>      case PHYSDEVOP_unmap_pirq:
>> +        if ( is_hardware_domain(currd) )
>> +            break;
> 
> Also Jan already pointed this out in v2: this hypercall needs to be
> limited so a PVH dom0 cannot execute it against itself.  IOW: refuse
> the hypercall if DOMID_SELF or the passed domid matches the current
> domain domid.
Yes, I remember Jan's suggestion, but since the latest patch(the second patch 
of v3 on kernel side) has change the implementation, it does setup_gsi and 
map_pirq for dom0 itself, so I didn't add the DOMID_SELF check.

> 
> Thanks, Roger.

-- 
Best regards,
Jiqian Chen.