Re: [Xen-devel] [PATCH V2 19/25] x86/vioapic: extend vioapic_get_vector() to support remapping format RTE

[Lan Tianyu <tianyu.lan@xxxxxxxxx>]

On 2017年08月23日 18:14, Roger Pau Monné wrote:
> On Wed, Aug 09, 2017 at 04:34:20PM -0400, Lan Tianyu wrote:
>> From: Chao Gao <chao.gao@xxxxxxxxx>
>>
>> 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@xxxxxxxxx>
>> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
>> ---
>>  xen/arch/x86/hvm/vioapic.c | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
>> index 322f33c..ff0742d 100644
>> --- a/xen/arch/x86/hvm/vioapic.c
>> +++ b/xen/arch/x86/hvm/vioapic.c
>> @@ -565,11 +565,27 @@ 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 IO_APIC_route_remap_entry rte = { { vioapic->redirtbl[pin].bits 
>> } };
> 
> Designated initialization and const.
> 
>>  
>>      if ( !vioapic )
>>          return -EINVAL;
>>  
>> -    return vioapic->redirtbl[pin].fields.vector;
>> +    if ( rte.format )
>> +    {
>> +        int err;
>> +        struct irq_remapping_request request;
>> +        struct irq_remapping_info info;
>> +
>> +        irq_request_ioapic_fill(&request, vioapic->id, rte.val);
>> +        /* Currently, only viommu 0 is supported */
> 
> This seems to be hardcoded in a bunch of places, which makes me wonder
> whether having an array of vIOMMUs is the correct choice. I think that
> you should remove the array and have a single vIOMMU per domain.

The array is reserved for mult-vIOMMU support but so far no such
requirement as I know. In design stage, someone commented we should take
mult-vIOMMU support into account.

We may add callback of getting vIOMMU in vIOMMU ops and let vIOMMU
device model return associated vIOMMU instance according irq remapping
information(e.g source id). One VM suppose to have only one vIOMMU type.
When add multi-vIOMMU support, this logic also can be applied.

For current scenario. device model should return the first vIOMMU directly.

> 
>> +        err = viommu_get_irq_info(vioapic->domain, 0, &request, &info);
>> +        return !err ? info.vector : -1;
> 
> maybe:
> 
> return err ?: info.vector;
> 
> ?
> 
>> +    }
>> +    else
>> +    {
>> +        return vioapic->redirtbl[pin].fields.vector;
>> +    }
>> +
>>  }
>>  
>>  int vioapic_get_trigger_mode(const struct domain *d, unsigned int gsi)
>> -- 
>> 1.8.3.1
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxx
>> https://lists.xen.org/xen-devel


-- 
Best regards
Tianyu Lan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel