tools/libxl/libxl_arm.c | 42 ++++++++++++++++++++++++++++++------- tools/libxl/libxl_types.idl | 1 + tools/xl/xl_parse.c | 7 +++++++ 3 files changed, 42 insertions(+), 8 deletions(-)
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
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.
Specify in domain configuration file which values should be added
by providing "dtb_compatible" list of strings separated by comas.
Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
tools/libxl/libxl_arm.c | 42 ++++++++++++++++++++++++++++++-------
tools/libxl/libxl_types.idl | 1 +
tools/xl/xl_parse.c | 7 +++++++
3 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index bf31b9b3ca..b956a6356c 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -270,20 +270,46 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
static int make_root_properties(libxl__gc *gc,
const libxl_version_info *vers,
- void *fdt)
+ void *fdt,
+ const libxl_domain_build_info *info)
{
- int res;
+ const char *compat0 = GCSPRINTF("xen,xenvm-%d.%d",
+ vers->xen_version_major,
+ vers->xen_version_minor);
+ const char *compat1 = "xen,xenvm";
+ const char **compats;
+ char *compat, *p;
+ size_t sz = 0;
+ int i, res, num_compats;
res = fdt_property_string(fdt, "model", GCSPRINTF("XENVM-%d.%d",
vers->xen_version_major,
vers->xen_version_minor));
if (res) return res;
- res = fdt_property_compat(gc, fdt, 2,
- GCSPRINTF("xen,xenvm-%d.%d",
- vers->xen_version_major,
- vers->xen_version_minor),
- "xen,xenvm");
+ num_compats = 2 + libxl_string_list_length(&info->dt_compatible);
+ compats = libxl__zalloc(gc, num_compats * sizeof(*compats));
+ if (!compats)
+ return -FDT_ERR_INTERNAL;
+
+ compats[0] = compat0;
+ compats[1] = compat1;
+ sz = strlen(compat0) + strlen(compat1) + 2;
+ for (i = 0; info->dt_compatible && info->dt_compatible[i] != NULL; i++) {
+ compats[2 + i] = info->dt_compatible[i];
+ sz += strlen(info->dt_compatible[i]) + 1;
+ }
+
+ p = compat = libxl__zalloc(gc, sz);
+ if (!p)
+ return -FDT_ERR_INTERNAL;
+
+ for (i = 0; i < num_compats; i++) {
+ strcpy(p, compats[i]);
+ p += strlen(compats[i]) + 1;
+ }
+
+ res = fdt_property(fdt, "compatible", compat, sz);
if (res) return res;
res = fdt_property_cell(fdt, "interrupt-parent", GUEST_PHANDLE_GIC);
@@ -930,7 +956,7 @@ next_resize:
FDT( fdt_begin_node(fdt, "") );
- FDT( make_root_properties(gc, vers, fdt) );
+ FDT( make_root_properties(gc, vers, fdt, info) );
FDT( make_chosen_node(gc, fdt, !!dom->modules[0].blob, state, info) );
FDT( make_cpus_node(gc, fdt, info->max_vcpus, ainfo) );
FDT( make_psci_node(gc, fdt) );
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 3ac9494b80..08ffb65904 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -544,6 +544,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
# Note that the partial device tree should avoid to use the phandle
# 65000 which is reserved by the toolstack.
("device_tree", string),
+ ("dt_compatible", libxl_string_list),
("acpi", libxl_defbool),
("bootloader", string),
("bootloader_args", libxl_string_list),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 03a2c54dd2..db9821c765 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2408,6 +2408,13 @@ skip_vfb:
}
}
+ e = xlu_cfg_get_list_as_string_list(config, "dt_compatible",
+ &b_info->dt_compatible, 1);
+ if (e && e != ESRCH) {
+ fprintf(stderr,"xl: Unable to parse dt_compatible\n");
+ exit(-ERROR_FAIL);
+ }
+
if (!xlu_cfg_get_list(config, "usbctrl", &usbctrls, 0, 0)) {
d_config->num_usbctrls = 0;
d_config->usbctrls = NULL;
--
2.17.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develOleksandr Grytsov writes ("[PATCH v1] libxl: Add DTB compatible list to config file"):
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> 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.
> 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 ?
As for the detail of the code:
The method you use for building compats[] is really rather ad-hoc.
Why not use a flexarray ?
(Also you do not need to check the error return from libxl__zalloc.
From libxl.h:
* Memory allocation failures are not handled gracefully. If malloc
* (or realloc) fails, libxl will cause the entire process to print
* a message to stderr and exit with status 255.
But really (i) you should be using GCNEW_ARRAY anyway and (ii) this
is all irrelevant if you switch to a flexarray instead.)
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
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@epam.com>
>>
>> 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.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
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@epam.com>
> > >
> > > 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?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Fri, Oct 11, 2019 at 8:21 PM Stefano Stabellini
<sstabellini@kernel.org> 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@epam.com>
> > > >
> > > > 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.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi Oleksandr,
On 16/10/2019 15:04, Oleksandr Grytsov wrote:
> On Fri, Oct 11, 2019 at 8:21 PM Stefano Stabellini
> <sstabellini@kernel.org> 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@epam.com>
>>>>>
>>>>> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Wed, Oct 16, 2019 at 5:12 PM Julien Grall <julien.grall@arm.com> wrote:
>
> Hi Oleksandr,
>
> On 16/10/2019 15:04, Oleksandr Grytsov wrote:
> > On Fri, Oct 11, 2019 at 8:21 PM Stefano Stabellini
> > <sstabellini@kernel.org> 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@epam.com>
> >>>>>
> >>>>> 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. 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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi,
On 16/10/2019 15:34, Oleksandr Grytsov wrote:
> On Wed, Oct 16, 2019 at 5:12 PM Julien Grall <julien.grall@arm.com> wrote:
>>
>> Hi Oleksandr,
>>
>> On 16/10/2019 15:04, Oleksandr Grytsov wrote:
>>> On Fri, Oct 11, 2019 at 8:21 PM Stefano Stabellini
>>> <sstabellini@kernel.org> 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@epam.com>
>>>>>>>
>>>>>>> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 16.10.19 18:04, Julien Grall wrote:
> Hi,
Hi Julien
just my 2 cents)
>
>> 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?
I have no idea whether it is ok or not to pass machine/SoC compatible to
a guest from the safety PoV, so I am not going to comment regarding safety.
I just would like to provide description of the problem we could face
when not passing machine/SoC compatible to a guest which runs real H/W
(not PV) devices.
...
I have just checked without passing real "dt_compatible" to a guest on
the M3N board. So, this can be considered as real example.
I noticed that at least two H/W devices (which are pass throughed to the
guest) suffered from the lack of compatible: sdhi_internal_dmac and
xhci-hcd. And as result SD card and USB host are not functional.
Why this happened? There is SoC Identification framework which purpose
is to detect SoC's id/revision for the future use in drivers to properly
apply some quirks (if needed). And without real compatible string in
place the framework fails
to proceed leaving the SoC attributes unregistered [1]. This results in
SDHI Internal DMAC controller fails to identify the SoC [2], so can't
operate.
I didn't investigate what is wrong with the xHCI, but I tend to think
that the problem is close to what we have with the SDHI.
[1]
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/soc/renesas/renesas-soc.c#L292
[2]
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/mmc/host/renesas_sdhi_internal_dmac.c#L328
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 23/10/2019 17:11, Oleksandr wrote:
>
> On 16.10.19 18:04, Julien Grall wrote:
Hi,
>>> 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?
>
> I have no idea whether it is ok or not to pass machine/SoC compatible to a guest
> from the safety PoV, so I am not going to comment regarding safety.
> I just would like to provide description of the problem we could face when not
> passing machine/SoC compatible to a guest which runs real H/W (not PV) devices.
>
> ...
>
> I have just checked without passing real "dt_compatible" to a guest on the M3N
> board. So, this can be considered as real example.
> I noticed that at least two H/W devices (which are pass throughed to the guest)
> suffered from the lack of compatible: sdhi_internal_dmac and xhci-hcd. And as
> result SD card and USB host are not functional.
> Why this happened? There is SoC Identification framework which purpose is to
> detect SoC's id/revision for the future use in drivers to properly apply some
> quirks (if needed). And without real compatible string in place the framework fails
> to proceed leaving the SoC attributes unregistered [1]. This results in SDHI
> Internal DMAC controller fails to identify the SoC [2], so can't operate.
Thank you for giving more information on the problem.
Usually when you have quirks required for a device, they will be the same for
all platforms using the same device revision. So it feels a bit odd to base it
on a SoC ID/revision.
The problem you described above would also happen if you move to a new SoC with
the same device revision. You will need to update Linux in order to use that device.
Xen VM is comparable to a "SoC". For instance, we needed to add Xen knowledge in
Linux so it can boot. In the case of Device assignment, you can view this as a
derivation of Xen VM (let's call it "Xen VM Bar").
As for any new SoC, if you want your OS to function on "Xen VM Bar", you may be
required to modify it.
The approach suggested in this patch may work for you, but I don't think this is
an approach that should be taken by Xen upstream. Instead, we should work with
the community to replace quirks based on SoC/ID with quirks based on device
binding property.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.