diff mbox series

[XEN,v11,6/8] tools/libxc: Allow gsi be mapped into a free pirq

Message ID 20240630123344.20623-7-Jiqian.Chen@amd.com (mailing list archive)
State Superseded
Headers show
Series Support device passthrough when dom0 is PVH on Xen | expand

Commit Message

Chen, Jiqian June 30, 2024, 12:33 p.m. UTC
Hypercall PHYSDEVOP_map_pirq support to map a gsi into a specific
pirq or a free pirq, it depends on the parameter pirq(>0 or <0).
But in current xc_physdev_map_pirq, it set *pirq=index when
parameter pirq is <0, it causes to force all cases to be mapped
to a specific pirq. That has some problems, one is caller can't
get a free pirq value, another is that once the pecific pirq was
already mapped to other gsi, then it will fail.

So, change xc_physdev_map_pirq to allow to pass negative parameter
in and then get a free pirq.

Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
 tools/libs/ctrl/xc_physdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Beulich July 1, 2024, 7:54 a.m. UTC | #1
On 30.06.2024 14:33, Jiqian Chen wrote:
> Hypercall PHYSDEVOP_map_pirq support to map a gsi into a specific
> pirq or a free pirq, it depends on the parameter pirq(>0 or <0).
> But in current xc_physdev_map_pirq, it set *pirq=index when
> parameter pirq is <0, it causes to force all cases to be mapped
> to a specific pirq. That has some problems, one is caller can't
> get a free pirq value, another is that once the pecific pirq was
> already mapped to other gsi, then it will fail.
> 
> So, change xc_physdev_map_pirq to allow to pass negative parameter
> in and then get a free pirq.
> 
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> ---
>  tools/libs/ctrl/xc_physdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libs/ctrl/xc_physdev.c b/tools/libs/ctrl/xc_physdev.c
> index 460a8e779ce8..e9fcd755fa62 100644
> --- a/tools/libs/ctrl/xc_physdev.c
> +++ b/tools/libs/ctrl/xc_physdev.c
> @@ -50,7 +50,7 @@ int xc_physdev_map_pirq(xc_interface *xch,
>      map.domid = domid;
>      map.type = MAP_PIRQ_TYPE_GSI;
>      map.index = index;
> -    map.pirq = *pirq < 0 ? index : *pirq;
> +    map.pirq = *pirq;
>  
>      rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));
>  

This is a functional change to existing callers, without any kind of
clarification whether this changed behavior is actually okay for them.

Jan
Chen, Jiqian July 2, 2024, 3:41 a.m. UTC | #2
On 2024/7/1 15:54, Jan Beulich wrote:
> On 30.06.2024 14:33, Jiqian Chen wrote:
>> Hypercall PHYSDEVOP_map_pirq support to map a gsi into a specific
>> pirq or a free pirq, it depends on the parameter pirq(>0 or <0).
>> But in current xc_physdev_map_pirq, it set *pirq=index when
>> parameter pirq is <0, it causes to force all cases to be mapped
>> to a specific pirq. That has some problems, one is caller can't
>> get a free pirq value, another is that once the pecific pirq was
>> already mapped to other gsi, then it will fail.
>>
>> So, change xc_physdev_map_pirq to allow to pass negative parameter
>> in and then get a free pirq.
>>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>> ---
>>  tools/libs/ctrl/xc_physdev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/libs/ctrl/xc_physdev.c b/tools/libs/ctrl/xc_physdev.c
>> index 460a8e779ce8..e9fcd755fa62 100644
>> --- a/tools/libs/ctrl/xc_physdev.c
>> +++ b/tools/libs/ctrl/xc_physdev.c
>> @@ -50,7 +50,7 @@ int xc_physdev_map_pirq(xc_interface *xch,
>>      map.domid = domid;
>>      map.type = MAP_PIRQ_TYPE_GSI;
>>      map.index = index;
>> -    map.pirq = *pirq < 0 ? index : *pirq;
>> +    map.pirq = *pirq;
>>  
>>      rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));
>>  
> 
> This is a functional change to existing callers, without any kind of
> clarification whether this changed behavior is actually okay for them.
Make sense.
There are three callers pci_add_dm_done, libxl__arch_domain_map_irq and pyxc_physdev_map_pirq,
I know how to clarify the first two, but the last one, I have no idea.

Hi Marek,
Will this patch break the existing behavior of pyxc_physdev_map_pirq, and why?

> 
> Jan
diff mbox series

Patch

diff --git a/tools/libs/ctrl/xc_physdev.c b/tools/libs/ctrl/xc_physdev.c
index 460a8e779ce8..e9fcd755fa62 100644
--- a/tools/libs/ctrl/xc_physdev.c
+++ b/tools/libs/ctrl/xc_physdev.c
@@ -50,7 +50,7 @@  int xc_physdev_map_pirq(xc_interface *xch,
     map.domid = domid;
     map.type = MAP_PIRQ_TYPE_GSI;
     map.index = index;
-    map.pirq = *pirq < 0 ? index : *pirq;
+    map.pirq = *pirq;
 
     rc = do_physdev_op(xch, PHYSDEVOP_map_pirq, &map, sizeof(map));