Re: [Xen-devel] [RFC PATCH v1 02/10] xen/arm: register mmio handler at runtime

[Vijay Kilari <vijay.kilari@xxxxxxxxx>]

On Tue, Apr 1, 2014 at 4:30 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> On 04/01/2014 10:34 AM, Vijay Kilari wrote:
>> Hi Julien,
>
> Hello Vijay,
>
>> On Thu, Mar 27, 2014 at 8:32 PM, Julien Grall <julien.grall@xxxxxxxxxx> 
>> wrote:
>>> On 03/27/2014 05:40 AM, Vijay Kilari wrote:
>>>> On Wed, Mar 26, 2014 at 8:17 PM, Julien Grall <julien.grall@xxxxxxxxxx> 
>>>> wrote:
>>>>> On 03/26/2014 12:29 PM, Vijay Kilari wrote:
>>>>>> Hi Julien,
>>>>>>
>>>>>>
>>>>>> On Fri, Mar 21, 2014 at 10:53 PM, Julien Grall <julien.grall@xxxxxxxxxx> 
>>>>>> wrote:
>>>>>>> On 03/21/2014 05:17 PM, Ian Campbell wrote:
>>>>>>>> On Wed, 2014-03-19 at 19:47 +0530, vijay.kilari@xxxxxxxxx wrote:
>>>>>>>>> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
>>>>>>>>>
>>>>>>>>> mmio handlers are registers at compile time
>>>>>>>>> for drivers like vuart and vgic.
>>>>>>>>> Make mmio handler registered at runtime by
>>>>>>>>> creating linked list of mmio handlers
>>>>>>>>
>>>>>>>> I'm not convinced of the need for this, certainly the vgic side can 
>>>>>>>> just
>>>>>>>> demux into v2 or v3 as necessary.
>>>>>>>
>>>>>>> Demux the code just add an indirection. We could have a list of mmio
>>>>>>> handler per domain and rely on it to call the right handler. A bit like 
>>>>>>> x86.
>>>>>>>
>>>>>>  Until Andrii adds IOMMU  handling should keep this patch? and adopt
>>>>>> to it later?
>>>>>
>>>>> I'm not sure to understand. IHMO, it doesn't sound right to upstream a
>>>>> patch that we know it will be superseded in a couple of months. Why
>>>>> can't you rework your patch to have it in good shape now?
>>>>>
>>>>  I assume that Andii's patch will be required to make mmio handlers
>>>> per domain. If not let me know references in x86 that I can make it
>>>> per domain specific mmio handlers.
>>>
>>> I have already pointed out to the x86 code few mails before.
>>>
>>> You can look at xen/arch/x86/intercept.c.
>>
>>    From the x86/hvm/intercept.c file, the mmio handling of x86 is
>> similar to existing arm mmio handling in arm/io.c file, where the
>> handlers are registered statically.
>
> Not really, x86 use an hybrid approach:
>     - static handler for IO region that are handled by every guest (hpet
> ...)
>      - dynamic handler registered by register_io_handler
>
I think, I can still keep dynamic handler registration.

>>   I understand that only requirement is to make vgic as domain specific
>> similar to vuart. Is that what you expect. Can you please make it clear?
>
> vuart and vgic handler won't be the only dynamic handlers. On some
> platform (e.g the cubieboard) we will have to use a similar solution to
> be able to passthrough UARTs to dom0. Currently they are blacklisted
> because the UART used by Xen shares the page with the other UARTs.
>
> Using the x86 approach will allow us to register the right vGIC
> emulation for every guest. We might want to start a shiny FreeBSD guest
> with GICv2 emulation and a Linux guest with a GICv3.
>
   To some extent vgic is domain specific because vgic structure defined
within arch_domain holds address of vgic with which check_handler is
comparing against trapped address (gpa)

   However when domain is created it does not check for Dom's GIC
supported version.
From arch_domain_create() gicv_setup() & domain_vgic_init() is
initializing GICv2 version
always.

With introduction of GICv3, now gicv_setup() & domain_vgic_init()
should able to
identify dom's gic version and initialize accordingly.
Is there any way to know what is Dom's/guest support GIC version?.

Or will below ensure picking right version of vgic?. I just had a doubt that
dt_host always picks dom0's/Xen's device tree.

dt_for_each_device_node(dt_host, node)
{
       rc = device_init(node, DEVICE_VGIC, NULL);
        if ( !rc )
            num_vgics++;
}

>>
>>  IMO, the io handling in x86/hvm/intercept.c is not required for arm.
>
> IHMO, as MMIO is often trapped, the performance is critical. Your
> solution won't improve the current implementation because you still have
> to browse unused MMIO handler each time the guest will trap into Xen.
>
> Regards,
>
> --
> Julien Grall

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