diff mbox

[V3,22/29] x86/vioapic: extend vioapic_get_vector() to support remapping format RTE

Message ID 1506049330-11196-23-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu Sept. 22, 2017, 3:02 a.m. UTC
From: Chao Gao <chao.gao@intel.com>

When IOAPIC RTE is in remapping format, it doesn't contain the vector of
interrupt. For this case, the RTE contains an index of interrupt remapping
table where the vector of interrupt is stored. This patchs gets the vector
through a vIOMMU interface.

Signed-off-by: Chao Gao <chao.gao@intel.com>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 xen/arch/x86/hvm/vioapic.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Roger Pau Monné Oct. 19, 2017, 3:49 p.m. UTC | #1
On Thu, Sep 21, 2017 at 11:02:03PM -0400, Lan Tianyu wrote:
> From: Chao Gao <chao.gao@intel.com>
> 
> When IOAPIC RTE is in remapping format, it doesn't contain the vector of
> interrupt. For this case, the RTE contains an index of interrupt remapping
> table where the vector of interrupt is stored. This patchs gets the vector
> through a vIOMMU interface.
> 
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  xen/arch/x86/hvm/vioapic.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
> index 5d0d1cd..9e47ef4 100644
> --- a/xen/arch/x86/hvm/vioapic.c
> +++ b/xen/arch/x86/hvm/vioapic.c
> @@ -561,11 +561,25 @@ int vioapic_get_vector(const struct domain *d, unsigned int gsi)
>  {
>      unsigned int pin;
>      const struct hvm_vioapic *vioapic = gsi_vioapic(d, gsi, &pin);
> +    struct arch_irq_remapping_request request;
>  
>      if ( !vioapic )
>          return -EINVAL;
>  
> -    return vioapic->redirtbl[pin].fields.vector;
> +    irq_request_ioapic_fill(&request, vioapic->id, vioapic->redirtbl[pin].bits);
> +    if ( viommu_check_irq_remapping(vioapic->domain, &request) )
> +    {
> +        int err;
> +        struct arch_irq_remapping_info info;
> +
> +        err = viommu_get_irq_info(vioapic->domain, &request, &info);
> +        return !err ? info.vector : err;

You can simplify this as return err :? info.vector;

Roger.
Jan Beulich Oct. 19, 2017, 3:56 p.m. UTC | #2
>>> On 19.10.17 at 17:49, <roger.pau@citrix.com> wrote:
> On Thu, Sep 21, 2017 at 11:02:03PM -0400, Lan Tianyu wrote:
>> --- a/xen/arch/x86/hvm/vioapic.c
>> +++ b/xen/arch/x86/hvm/vioapic.c
>> @@ -561,11 +561,25 @@ int vioapic_get_vector(const struct domain *d, unsigned int gsi)
>>  {
>>      unsigned int pin;
>>      const struct hvm_vioapic *vioapic = gsi_vioapic(d, gsi, &pin);
>> +    struct arch_irq_remapping_request request;
>>  
>>      if ( !vioapic )
>>          return -EINVAL;
>>  
>> -    return vioapic->redirtbl[pin].fields.vector;
>> +    irq_request_ioapic_fill(&request, vioapic->id, vioapic->redirtbl[pin].bits);
>> +    if ( viommu_check_irq_remapping(vioapic->domain, &request) )
>> +    {
>> +        int err;
>> +        struct arch_irq_remapping_info info;
>> +
>> +        err = viommu_get_irq_info(vioapic->domain, &request, &info);
>> +        return !err ? info.vector : err;
> 
> You can simplify this as return err :? info.vector;

At which point the local variable becomes pretty pointless.

Jan
Chao Gao Oct. 20, 2017, 1:04 a.m. UTC | #3
On Thu, Oct 19, 2017 at 09:56:34AM -0600, Jan Beulich wrote:
>>>> On 19.10.17 at 17:49, <roger.pau@citrix.com> wrote:
>> On Thu, Sep 21, 2017 at 11:02:03PM -0400, Lan Tianyu wrote:
>>> --- a/xen/arch/x86/hvm/vioapic.c
>>> +++ b/xen/arch/x86/hvm/vioapic.c
>>> @@ -561,11 +561,25 @@ int vioapic_get_vector(const struct domain *d, unsigned int gsi)
>>>  {
>>>      unsigned int pin;
>>>      const struct hvm_vioapic *vioapic = gsi_vioapic(d, gsi, &pin);
>>> +    struct arch_irq_remapping_request request;
>>>  
>>>      if ( !vioapic )
>>>          return -EINVAL;
>>>  
>>> -    return vioapic->redirtbl[pin].fields.vector;
>>> +    irq_request_ioapic_fill(&request, vioapic->id, vioapic->redirtbl[pin].bits);
>>> +    if ( viommu_check_irq_remapping(vioapic->domain, &request) )
>>> +    {
>>> +        int err;
>>> +        struct arch_irq_remapping_info info;
>>> +
>>> +        err = viommu_get_irq_info(vioapic->domain, &request, &info);
>>> +        return !err ? info.vector : err;
>> 
>> You can simplify this as return err :? info.vector;
>
>At which point the local variable becomes pretty pointless.

Maybe we can remove 'err' and return
unlikely(viommu_get_irq_info(...)) ?: info.vector;

Thanks
Chao
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
index 5d0d1cd..9e47ef4 100644
--- a/xen/arch/x86/hvm/vioapic.c
+++ b/xen/arch/x86/hvm/vioapic.c
@@ -561,11 +561,25 @@  int vioapic_get_vector(const struct domain *d, unsigned int gsi)
 {
     unsigned int pin;
     const struct hvm_vioapic *vioapic = gsi_vioapic(d, gsi, &pin);
+    struct arch_irq_remapping_request request;
 
     if ( !vioapic )
         return -EINVAL;
 
-    return vioapic->redirtbl[pin].fields.vector;
+    irq_request_ioapic_fill(&request, vioapic->id, vioapic->redirtbl[pin].bits);
+    if ( viommu_check_irq_remapping(vioapic->domain, &request) )
+    {
+        int err;
+        struct arch_irq_remapping_info info;
+
+        err = viommu_get_irq_info(vioapic->domain, &request, &info);
+        return !err ? info.vector : err;
+    }
+    else
+    {
+        return vioapic->redirtbl[pin].fields.vector;
+    }
+
 }
 
 int vioapic_get_trigger_mode(const struct domain *d, unsigned int gsi)