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月24日 14:59, Jan Beulich wrote:
>>>> On 24.08.17 at 08:11, <tianyu.lan@xxxxxxxxx> wrote:
>> On 2017年08月23日 18:14, Roger Pau Monné wrote:
>>> On Wed, Aug 09, 2017 at 04:34:20PM -0400, Lan Tianyu wrote:
>>>> --- 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.
> 
> It _may_ suffice to do so at the public interface level. I'm not
> against using a single entry array right away, but then the rest
> of the code needs to be written as if the array bound was not
> fixed at 1, i.e. no hard coded uses of zero as the only valid
> array index should occur.

Hi Jan:

I am not sure whether we can hide single vIOMMU logic in the device
model. When vIOMMU instance is created in device model, store vIOMMU
instance in the global variable of virtual VTD code. Provide getting
viommu instance callback in vIOMMU ops and helper function in vIOMMU
abstract layer with interrupt information as parameter. New callback
always returns stored vIOMMU instance. This seems to avoid hard coded
uses of zero.

If this can't be accept, removing the array seems to be only feasible way.

-- 
Best regards
Tianyu Lan

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