diff mbox series

[v1,13/14] xen/arm: Fixed error when PCI device is assigned to guest

Message ID 917720808121c3098690b51d55f2d60118ec6408.1629366665.git.rahul.singh@arm.com (mailing list archive)
State Superseded
Headers show
Series PCI devices passthrough on Arm | expand

Commit Message

Rahul Singh Aug. 19, 2021, 12:02 p.m. UTC
XEN_DOMCTL_ioport_permission, PHYSDEVOP_unmap_pirq, PHYSDEVOP_unmap_pirq
are unimplemented for ARM. When libxl assigning a PCI device to the
guest error is observed related to above functions.

Implement dummy functions to fix the error.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
 xen/arch/arm/domctl.c  | 2 ++
 xen/arch/arm/physdev.c | 3 +++
 2 files changed, 5 insertions(+)

Comments

Jan Beulich Aug. 19, 2021, 12:12 p.m. UTC | #1
On 19.08.2021 14:02, Rahul Singh wrote:
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -173,6 +173,8 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
>  
>          return rc;
>      }
> +    case XEN_DOMCTL_ioport_permission:
> +        return 0;

I don't think returning success for something that doesn't make
much sense in the first place (there aren't truly "I/O ports" on
Arm afaik) is a good idea. Instead I think the tool stack should
avoid making arch-specific calls in an arch-independent way.

> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -42,6 +42,9 @@ int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>      }
>  #endif
> +    case PHYSDEVOP_unmap_pirq:
> +    case PHYSDEVOP_map_pirq:
> +        break;

Less sure here, but I'm not convinced either.

Jan
Julien Grall Aug. 19, 2021, 12:40 p.m. UTC | #2
Hi,

On 19/08/2021 13:12, Jan Beulich wrote:
> On 19.08.2021 14:02, Rahul Singh wrote:
>> --- a/xen/arch/arm/domctl.c
>> +++ b/xen/arch/arm/domctl.c
>> @@ -173,6 +173,8 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
>>   
>>           return rc;
>>       }
>> +    case XEN_DOMCTL_ioport_permission:
>> +        return 0;
> 
> I don't think returning success for something that doesn't make
> much sense in the first place (there aren't truly "I/O ports" on
> Arm afaik) is a good idea.

They are memory mapped. IIRC, there is a region that is reserved to fake 
them (for PCI cards that still using them).

> Instead I think the tool stack should
> avoid making arch-specific calls in an arch-independent way.

+1. The current approach means that it will be impossible to implement 
those sub operations without breaking Xen if we ever needs it.

Cheers,
Rahul Singh Aug. 20, 2021, 5:01 p.m. UTC | #3
Hi Jan,

> On 19 Aug 2021, at 1:12 pm, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 19.08.2021 14:02, Rahul Singh wrote:
>> --- a/xen/arch/arm/domctl.c
>> +++ b/xen/arch/arm/domctl.c
>> @@ -173,6 +173,8 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
>> 
>>         return rc;
>>     }
>> +    case XEN_DOMCTL_ioport_permission:
>> +        return 0;
> 
> I don't think returning success for something that doesn't make
> much sense in the first place (there aren't truly "I/O ports" on
> Arm afaik) is a good idea.
> Instead I think the tool stack should
> avoid making arch-specific calls in an arch-independent way.

I agree with you let me try to modify the toolstack not to call the arch-specific call.

Regards,
Rahul
> 
>> --- a/xen/arch/arm/physdev.c
>> +++ b/xen/arch/arm/physdev.c
>> @@ -42,6 +42,9 @@ int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>         break;
>>     }
>> #endif
>> +    case PHYSDEVOP_unmap_pirq:
>> +    case PHYSDEVOP_map_pirq:
>> +        break;
> 
> Less sure here, but I'm not convinced either.
> 
> Jan
>
diff mbox series

Patch

diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index b7d27f37df..38813be893 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -173,6 +173,8 @@  long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
 
         return rc;
     }
+    case XEN_DOMCTL_ioport_permission:
+        return 0;
     default:
     {
         int rc;
diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
index ccce8f0eba..4a0affeada 100644
--- a/xen/arch/arm/physdev.c
+++ b/xen/arch/arm/physdev.c
@@ -42,6 +42,9 @@  int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }
 #endif
+    case PHYSDEVOP_unmap_pirq:
+    case PHYSDEVOP_map_pirq:
+        break;
     default:
         gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
         ret = -ENOSYS;