On 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.
On Linux kernel side, it calles PHYSDEVOP_setup_gsi for
passthrough devices to register gsi when dom0 is PVH.
So, add PHYSDEVOP_setup_gsi for above purpose.

Signed-off-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
 xen/arch/x86/hvm/hypercall.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index d49fb8b548a3..98e3c6b176ff 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -76,6 +76,11 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     case PHYSDEVOP_unmap_pirq:
         break;
 
+    case PHYSDEVOP_setup_gsi:
+        if ( !is_hardware_domain(currd) )
+            return -EOPNOTSUPP;
+        break;
+
     case PHYSDEVOP_eoi:
     case PHYSDEVOP_irq_status_query:
     case PHYSDEVOP_get_free_pirq:
-- 
2.34.1

On 16.05.2024 11:52, Jiqian Chen wrote:
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -76,6 +76,11 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>      case PHYSDEVOP_unmap_pirq:
>          break;
>  
> +    case PHYSDEVOP_setup_gsi:
> +        if ( !is_hardware_domain(currd) )
> +            return -EOPNOTSUPP;
> +        break;
> +
>      case PHYSDEVOP_eoi:
>      case PHYSDEVOP_irq_status_query:
>      case PHYSDEVOP_get_free_pirq:

Below here we have a hardware-domain-only block already. Any reason not
to add to there? Yes, that uses -ENOSYS. Imo that wants changing anyway,
but even without that to me it would seem more consistent overall to have
the new case simply added there.

Just to double check: Is there a respective Linux patch already (if so,
cross-linking the patches may be helpful)? Or does PVH Linux invoke the
sub-op already anyway, just that so far it fails?

Jan

On 2024/5/16 21:49, Jan Beulich wrote:
> On 16.05.2024 11:52, Jiqian Chen wrote:
>> --- a/xen/arch/x86/hvm/hypercall.c
>> +++ b/xen/arch/x86/hvm/hypercall.c
>> @@ -76,6 +76,11 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>      case PHYSDEVOP_unmap_pirq:
>>          break;
>>  
>> +    case PHYSDEVOP_setup_gsi:
>> +        if ( !is_hardware_domain(currd) )
>> +            return -EOPNOTSUPP;
>> +        break;
>> +
>>      case PHYSDEVOP_eoi:
>>      case PHYSDEVOP_irq_status_query:
>>      case PHYSDEVOP_get_free_pirq:
> 
> Below here we have a hardware-domain-only block already. Any reason not
> to add to there? Yes, that uses -ENOSYS. Imo that wants changing anyway,
> but even without that to me it would seem more consistent overall to have
> the new case simply added there.
Ah yes, I remembered you suggest me to use EOPNOTSUPP in v4, if change to ENOSYS is also fine, I will move to below in next version.

> 
> Just to double check: Is there a respective Linux patch already (if so,
> cross-linking the patches may be helpful)?
Yes, my corresponding kernel patch:
https://lore.kernel.org/lkml/20240515065011.13797-1-Jiqian.Chen@amd.com/T/#mc56b111562d7c67886da905e306f12b3ef8076b4 
Do you mean I need to add this link into the commit message once the kernel patch is accepted?
> Or does PVH Linux invoke the sub-op already anyway, just that so far it fails? 
> 
> Jan

-- 
Best regards,
Jiqian Chen.

On 17.05.2024 11:00, Chen, Jiqian wrote:
> On 2024/5/16 21:49, Jan Beulich wrote:
>> On 16.05.2024 11:52, Jiqian Chen wrote:
>>> --- a/xen/arch/x86/hvm/hypercall.c
>>> +++ b/xen/arch/x86/hvm/hypercall.c
>>> @@ -76,6 +76,11 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>      case PHYSDEVOP_unmap_pirq:
>>>          break;
>>>  
>>> +    case PHYSDEVOP_setup_gsi:
>>> +        if ( !is_hardware_domain(currd) )
>>> +            return -EOPNOTSUPP;
>>> +        break;
>>> +
>>>      case PHYSDEVOP_eoi:
>>>      case PHYSDEVOP_irq_status_query:
>>>      case PHYSDEVOP_get_free_pirq:
>>
>> Below here we have a hardware-domain-only block already. Any reason not
>> to add to there? Yes, that uses -ENOSYS. Imo that wants changing anyway,
>> but even without that to me it would seem more consistent overall to have
>> the new case simply added there.
> Ah yes, I remembered you suggest me to use EOPNOTSUPP in v4, if change to ENOSYS is also fine, I will move to below in next version.
> 
>>
>> Just to double check: Is there a respective Linux patch already (if so,
>> cross-linking the patches may be helpful)?
> Yes, my corresponding kernel patch:
> https://lore.kernel.org/lkml/20240515065011.13797-1-Jiqian.Chen@amd.com/T/#mc56b111562d7c67886da905e306f12b3ef8076b4 
> Do you mean I need to add this link into the commit message once the kernel patch is accepted?

Not necessarily the commit message itself, but below the --- marker.
If the kernel patch was accepted earlier than the Xen one (which imo it
better shouldn't be), you'd likely want to move to pointing at the
resulting commit.

Jan