[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [XEN PATCH v1] libxl: Add DTB compatible list to config file



Hi,

On 16/10/2019 15:34, Oleksandr Grytsov wrote:
On Wed, Oct 16, 2019 at 5:12 PM Julien Grall <julien.grall@xxxxxxx> wrote:

Hi Oleksandr,

On 16/10/2019 15:04, Oleksandr Grytsov wrote:
On Fri, Oct 11, 2019 at 8:21 PM Stefano Stabellini
<sstabellini@xxxxxxxxxx> wrote:

On Fri, 11 Oct 2019, Julien Grall wrote:
Hi,

On 11/10/2019 16:23, Ian Jackson wrote:
Oleksandr Grytsov writes ("[PATCH v1] libxl: Add DTB compatible list to
config file"):
From: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>

Some platforms need more compatible property values in device
tree root node in addition to "xen,xenvm-%d.%d" and "xen,xenvm"
values that are given by Xen by default.

I am pretty sure I have seen this patch a few years ago, but I can't find it
in my inbox. What is the exact problem here?

Specify in domain configuration file which values should be added
by providing "dtb_compatible" list of strings separated by comas.

Hi, thanks.

I don't have an opinion about the principle of this and would like to
hear from ARM folks about that.

Also, Stefano, Julien: should we be asking for a freeze exception for
this for 4.13 ?

I don't have enough context to understand the exact issue he is trying to
solve.

Same here. Is this patch needed because on some platform the OS checks
for the platform "model" (also known as "machine name") on device tree
before continuing or to trigger a difference behavior?

Yes, exactly.

I will redo the patch with Ian's comments if it is ok in general.

By specifying a different compatible (let say "renesas,r8a774a1"), then you
claim that your virtual machine is exactly the same as that board.

This means, the OS is free to assume that the memory layout and all the devices
are exactly the same. This is definitely not true as the virtual machine we
expose is specific to Xen.

So I don't think this is the correct approach here. Can you provide a real-life
example on why you need this?

Cheers,

--
Julien Grall

This is required for HW domains. Some drivers or initialization routines check
compatible.

So this suggest you will need to expose the compatible to multiple domains at the same time.

How is this even going to be safe? As I pointed out in my previous e-mail, if you set the compatible, then your OS is free to assume all the devices for that SoC are present and the memory layout is fixed.

Very likely, you are only going to expose a subset of the devices to each domains. So you either going to have a crash (if nothing were mapped at the address access) or write to the wrong device.

Below is example from linux kernel sources:

linux/sound/ppc/awacs.c:741:    if (of_machine_is_compatible("PowerBook3,1")
linux/sound/ppc/awacs.c:742:        ||
of_machine_is_compatible("PowerBook3,2")) {
linux/sound/ppc/awacs.c:770:#define IS_PM7500
(of_machine_is_compatible("AAPL,7500") \
linux/sound/ppc/awacs.c:771:        || of_machine_is_compatible("AAPL,8500") \
linux/sound/ppc/awacs.c:772:        || of_machine_is_compatible("AAPL,9500"))
...
linux/arch/arm/mach-omap2/pdata-quirks.c:703:        if
(of_machine_is_compatible(quirks->compatible)) {
linux/arch/arm/mach-omap2/pdata-quirks.c:717:    if
(of_machine_is_compatible("ti,omap2420") ||
linux/arch/arm/mach-omap2/pdata-quirks.c:718:
of_machine_is_compatible("ti,omap3"))
linux/arch/arm/mach-omap2/pdata-quirks.c:721:    if
(of_machine_is_compatible("ti,omap3"))
...

Also see [1]

[1] 
https://source.codeaurora.org/external/imx/imx-xen/commit/?h=imx_4.14.98_2.0.0_ga&id=6e58d478203639e71da3da69ffeae3fa5dc0197b

So this is a grep from Linux, I have already done that. What I am looking at is an exact description of your problem. Can you tell me what you are trying to passthrough? Can you also provide a pointer to the Linux code checking the compatible for your problem?

But speaking with various Linux folks, a device should really not rely on the SoC compatible to decide whether it needs to be initialized/requires quirk. The correct solution here is to fix your bindings/driver so they don't rely on it.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.