Re: [RFC 21/24] xen/arm: vexpress: Blacklist a list of board specific devices

[Julien Grall <julien.grall@linaro.org>]

On 08/22/2013 03:36 PM, Ian Campbell wrote:
> On Thu, 2013-08-22 at 15:24 +0100, Julien Grall wrote:
>> On 08/22/2013 03:00 PM, Ian Campbell wrote:
>>> On Fri, 2013-08-16 at 22:05 +0100, Julien Grall wrote:
>>>> On Versatile there is a bunch of devices that must not pass-through to any
>>>
>>> "there are a bunch of devices which must not be passed-through"
>>>
>>>> guest (power management and cache coherency devices).
>>>>
>>>> This commit also blacklist the HDLCD device because then is unable to correctly
>>>                                                           ^Linux?
>>>
>>>> map the framebuffer. Therefore, when Linux will try to access to the framebuffer,
>>>> Xen will receive a non-handled data access.
>>>
>>> Can/should this be conditional on whether Xen has console=hdlcd or not?
>>> Or does Xen use the device unconditionally if it exists? TBH I think it
>>> would be normal to prefer that Linux gets this device...
>>>
>>> (I'm unclear how this relates to memreserve as mention in the code
>>> comment)
>>
>> This issue is only when the HDLCD is used by Linux (not Xen). To specify
>> where the framebuffer lives in the memory, there is a property
>> "framebuffer" which contains address/size.
>> This regions must be reserved to avoid Linux/u-boot play with it. So the
>> DTS has a memreserve range with the same value. I don't really
>> understand all memreserve things but Xen must cope with it.
> 
> Yes, Xen needs to learn memreserve, it's used on midway too for example.

As I understand memreserve can only contains RAM region. Right?

>> If this device is mapped, Xen will receive a data abort because Linux
>> can't access to the framebuffer.
> 
> Because it wasn't part of the set of memory which we assigned to the
> guest?

Yes. The framebuffer steals RAM, in this case, the end of it.

> It seems like it would be hard to link an individual memsreserve to a
> particular device, at least not without device specific logic (e.g
> looking at the "framebuffer" property in this case).
> 
> Sounds like we might need list of compatible -> dom0_init_hook
> functions, with the appropriate hook called for each device which we
> pass through.

It could be a solution. In this case, we just need to update the
framebuffer property and the memreserve.

> 
> Ian.
> 
>>
>> This solution is not upstream (only in the Linaro tree). I don't know if
>> this driver works with the vanilla kernel.
>>
>>>
>>>>
>>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>>> ---
>>>>  xen/arch/arm/platforms/vexpress.c |   17 +++++++++++++++++
>>>>  1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/xen/arch/arm/platforms/vexpress.c b/xen/arch/arm/platforms/vexpress.c
>>>> index 8fc30c4..ece7bd9 100644
>>>> --- a/xen/arch/arm/platforms/vexpress.c
>>>> +++ b/xen/arch/arm/platforms/vexpress.c
>>>> @@ -125,9 +125,26 @@ static const char const *vexpress_dt_compat[] __initdata =
>>>>      NULL
>>>>  };
>>>>  
>>>> +static const struct dt_device_match vexpress_blacklist_dev[] __initconst =
>>>> +{
>>>> +    /* Cache Coherent Interconnect */
>>>> +    DT_MATCH_COMPATIBLE("arm,cci-400"),
>>>> +    DT_MATCH_COMPATIBLE("arm,cci-400-pmu"),
>>>> +    /* Video device
>>>> +     * TODO: remove it once memreserve is handled properly by Xen
>>>> +     */
>>>> +    DT_MATCH_COMPATIBLE("arm,hdlcd"),
>>>> +    /* Hardware power management */
>>>> +    DT_MATCH_COMPATIBLE("arm,vexpress-reset"),
>>>> +    DT_MATCH_COMPATIBLE("arm,vexpress-reboot"),
>>>> +    DT_MATCH_COMPATIBLE("arm,vexpress-shutdown"),
>>>> +    { /* sentinel */ },
>>>> +};
>>>> +
>>>>  PLATFORM_START(vexpress, "VERSATILE EXPRESS")
>>>>      .compatible = vexpress_dt_compat,
>>>>      .reset = vexpress_reset,
>>>> +    .blacklist_dev = vexpress_blacklist_dev,
>>>>  PLATFORM_END
>>>>  
>>>>  /*
>>>
>>>
>>
>>
> 
> 


-- 
Julien Grall