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

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

On 08/22/2013 04:02 PM, Ian Campbell wrote:
> On Thu, 2013-08-22 at 15:51 +0100, Julien Grall wrote:
>> 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?
> 
> ePAPR seems to suggest it is only for RAM, yes.
> 
>>> 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.
> 
> I think in this case we probably want to create a 1:1 mapping of the
> video ram? In case e.g. the firmware has setup attributes, or the LCD
> cannot use a different address etc...

Do we need to take into account in the dom0 memory?

For example, the amount of video RAM memory is 6Mb and the DOM0 memory
is 256Mb.

Does Xen must allocate?
	- 6Mb of 1:1 mapping
        - 250Mb of domain allocate page

>>
>>>
>>> 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