| Message ID | 20240131114952.305805-2-john.ernberg@actia.se (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Xen: ARM: Improved NXP iMX8 platform support | expand |
Hi, On 31/01/2024 11:50, John Ernberg wrote: > When using Linux for dom0 there are a bunch of drivers that need to do SMC > SIP calls into the PSCI provider to enable certain hardware bits like the > watchdog. Do you know which protocol this is under the hood. Is this SCMI? > > Provide a basic platform glue that implements the needed SMC forwarding. > > Signed-off-by: John Ernberg <john.ernberg@actia.se> > --- > NOTE: This is based on code found in NXP Xen tree located here: > https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c Anything after --- will be removed while applied to the three. I think this NOTE should be written down in the commit message. You also possibly want a signed-off-by from Peng as this is his code. > > xen/arch/arm/platforms/Makefile | 1 + > xen/arch/arm/platforms/imx8qm.c | 65 +++++++++++++++++++++++++++++++++ > 2 files changed, 66 insertions(+) > create mode 100644 xen/arch/arm/platforms/imx8qm.c > > diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile > index 8632f4115f..bec6e55d1f 100644 > --- a/xen/arch/arm/platforms/Makefile > +++ b/xen/arch/arm/platforms/Makefile > @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o > obj-$(CONFIG_ALL64_PLAT) += thunderx.o > obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o > obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o > +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o > obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o > obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o > diff --git a/xen/arch/arm/platforms/imx8qm.c b/xen/arch/arm/platforms/imx8qm.c > new file mode 100644 > index 0000000000..a9cd9c3615 > --- /dev/null > +++ b/xen/arch/arm/platforms/imx8qm.c > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * xen/arch/arm/platforms/imx8qm.c > + * > + * i.MX 8QM setup > + * > + * Copyright (c) 2016 Freescale Inc. > + * Copyright 2018-2019 NXP > + * > + * > + * Peng Fan <peng.fan@nxp.com> > + */ > + > +#include <asm/platform.h> > +#include <asm/smccc.h> > + > +static const char * const imx8qm_dt_compat[] __initconst = > +{ > + "fsl,imx8qm", > + "fsl,imx8qxp", > + NULL > +}; > + > +static bool imx8qm_smc(struct cpu_user_regs *regs) > +{ Your implementation below will not only forward SMC for dom0 but also for any non-trusted domains. Have you investigated that all the SIP calls are safe to be called by anyone? But even if we restrict to dom0, have you checked that none of the SMCs use buffers? Rather than providing a blanket forward, to me it sounds more like you want to provide an allowlist of the SMCs. This is more futureproof and avoid the risk to expose unsafe SMCs to any domain. For an example, you can have a look at the EEMI mediator for Xilinx. Cheers,
Hi Julien, On 1/31/24 13:22, Julien Grall wrote: > Hi, > > On 31/01/2024 11:50, John Ernberg wrote: >> When using Linux for dom0 there are a bunch of drivers that need to do >> SMC >> SIP calls into the PSCI provider to enable certain hardware bits like the >> watchdog. > > Do you know which protocol this is under the hood. Is this SCMI? I think I confused myself here when I wrote the commit log. The EL3 code in our case is ATF, and it does not appear to be SCMI, nor PSCI. The register usage of these SMC SIP calls are as follows: a0 - service a1 - function a2-a7 - args In ATF the handler is declared as a runtime service. Would the appropriate commmit message here be something along the lines of below? """ When using Linux for dom0 there are a bunch of drivers that need to do SMC SIP calls into the firmware to enable certain hardware bits like the watchdog. """ > >> >> Provide a basic platform glue that implements the needed SMC forwarding. >> >> Signed-off-by: John Ernberg <john.ernberg@actia.se> >> --- >> NOTE: This is based on code found in NXP Xen tree located here: >> https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c > > Anything after --- will be removed while applied to the three. I think > this NOTE should be written down in the commit message. Ack. > > You also possibly want a signed-off-by from Peng as this is his code. @Peng: May I add a sign-off from you? > >> >> xen/arch/arm/platforms/Makefile | 1 + >> xen/arch/arm/platforms/imx8qm.c | 65 +++++++++++++++++++++++++++++++++ >> 2 files changed, 66 insertions(+) >> create mode 100644 xen/arch/arm/platforms/imx8qm.c >> >> diff --git a/xen/arch/arm/platforms/Makefile >> b/xen/arch/arm/platforms/Makefile >> index 8632f4115f..bec6e55d1f 100644 >> --- a/xen/arch/arm/platforms/Makefile >> +++ b/xen/arch/arm/platforms/Makefile >> @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o >> obj-$(CONFIG_ALL64_PLAT) += thunderx.o >> obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o >> obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o >> +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o >> diff --git a/xen/arch/arm/platforms/imx8qm.c >> b/xen/arch/arm/platforms/imx8qm.c >> new file mode 100644 >> index 0000000000..a9cd9c3615 >> --- /dev/null >> +++ b/xen/arch/arm/platforms/imx8qm.c >> @@ -0,0 +1,65 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> +/* >> + * xen/arch/arm/platforms/imx8qm.c >> + * >> + * i.MX 8QM setup >> + * >> + * Copyright (c) 2016 Freescale Inc. >> + * Copyright 2018-2019 NXP >> + * >> + * >> + * Peng Fan <peng.fan@nxp.com> >> + */ >> + >> +#include <asm/platform.h> >> +#include <asm/smccc.h> >> + >> +static const char * const imx8qm_dt_compat[] __initconst = >> +{ >> + "fsl,imx8qm", >> + "fsl,imx8qxp", >> + NULL >> +}; >> + >> +static bool imx8qm_smc(struct cpu_user_regs *regs) >> +{ > > Your implementation below will not only forward SMC for dom0 but also > for any non-trusted domains. Have you investigated that all the SIP > calls are safe to be called by anyone? We use pure virtualized domUs, so we do not expect any calls to this SMC interface from the guest. I'll limit it to dom0. > > But even if we restrict to dom0, have you checked that none of the SMCs > use buffers? I haven't found any such instances in the Linux kernel where a buffer is used. Adding a call filtering like suggested below additions of such functions can be discovered and adapted for if they would show up later. > > Rather than providing a blanket forward, to me it sounds more like you > want to provide an allowlist of the SMCs. This is more futureproof and > avoid the risk to expose unsafe SMCs to any domain. > > For an example, you can have a look at the EEMI mediator for Xilinx. Ack. Do you prefer to see only on SMCCC service level or also on function level? (a1 register, per description earlier) > > Cheers, > Thanks! // John Ernberg
> Cc: Jonas Blixt <jonas.blixt@actia.se>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue > > Hi Julien, > > On 1/31/24 13:22, Julien Grall wrote: > > Hi, > > > > On 31/01/2024 11:50, John Ernberg wrote: > >> When using Linux for dom0 there are a bunch of drivers that need to > >> do SMC SIP calls into the PSCI provider to enable certain hardware > >> bits like the watchdog. > > > > Do you know which protocol this is under the hood. Is this SCMI? > > I think I confused myself here when I wrote the commit log. > > The EL3 code in our case is ATF, and it does not appear to be SCMI, nor PSCI. > The register usage of these SMC SIP calls are as follows: > a0 - service > a1 - function > a2-a7 - args > > In ATF the handler is declared as a runtime service. > > Would the appropriate commmit message here be something along the lines > of below? > """ > When using Linux for dom0 there are a bunch of drivers that need to do > SMC > SIP calls into the firmware to enable certain hardware bits like the watchdog. > """ > > > >> > >> Provide a basic platform glue that implements the needed SMC forwarding. > >> > >> Signed-off-by: John Ernberg <john.ernberg@actia.se> > >> --- > >> NOTE: This is based on code found in NXP Xen tree located here: > >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit > >> hub.com%2Fnxp-imx%2Fimx-xen%2Fblob%2Flf- > 5.10.y_4.13%2Fxen%2Farch%2Far > >> > m%2Fplatforms%2Fimx8qm.c&data=05%7C02%7Cpeng.fan%40nxp.com%7C > 573b599a > >> > 4b4143ceca1d08dc2271e5be%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C > 0%7C0%7 > >> > C638423119777601548%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA > wMDAiLCJQI > >> > joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=ZO > 0TXjL6 > >> g0W7TIZo8x8lTNBXEZW%2BDNcLPndWlEf5D2A%3D&reserved=0 > > > > Anything after --- will be removed while applied to the three. I think > > this NOTE should be written down in the commit message. > > Ack. > > > > You also possibly want a signed-off-by from Peng as this is his code. > > @Peng: May I add a sign-off from you? Yeah. You could add my sign off. > > > >> > >> xen/arch/arm/platforms/Makefile | 1 + > >> xen/arch/arm/platforms/imx8qm.c | 65 > >> +++++++++++++++++++++++++++++++++ > >> 2 files changed, 66 insertions(+) > >> create mode 100644 xen/arch/arm/platforms/imx8qm.c > >> > >> diff --git a/xen/arch/arm/platforms/Makefile > >> b/xen/arch/arm/platforms/Makefile index 8632f4115f..bec6e55d1f > 100644 > >> --- a/xen/arch/arm/platforms/Makefile > >> +++ b/xen/arch/arm/platforms/Makefile > >> @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o > >> obj-$(CONFIG_ALL64_PLAT) += thunderx.o > >> obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o > >> obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o > >> +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o > >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o > >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o diff --git > >> a/xen/arch/arm/platforms/imx8qm.c > b/xen/arch/arm/platforms/imx8qm.c > >> new file mode 100644 index 0000000000..a9cd9c3615 > >> --- /dev/null > >> +++ b/xen/arch/arm/platforms/imx8qm.c > >> @@ -0,0 +1,65 @@ > >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ > >> +/* > >> + * xen/arch/arm/platforms/imx8qm.c > >> + * > >> + * i.MX 8QM setup > >> + * > >> + * Copyright (c) 2016 Freescale Inc. > >> + * Copyright 2018-2019 NXP > >> + * > >> + * > >> + * Peng Fan <peng.fan@nxp.com> > >> + */ > >> + > >> +#include <asm/platform.h> > >> +#include <asm/smccc.h> > >> + > >> +static const char * const imx8qm_dt_compat[] __initconst = { > >> + "fsl,imx8qm", > >> + "fsl,imx8qxp", > >> + NULL > >> +}; > >> + > >> +static bool imx8qm_smc(struct cpu_user_regs *regs) { > > > > Your implementation below will not only forward SMC for dom0 but also > > for any non-trusted domains. Have you investigated that all the SIP > > calls are safe to be called by anyone? > > We use pure virtualized domUs, so we do not expect any calls to this SMC > interface from the guest. I'll limit it to dom0. Would you mind to share what features are supported in your DomU? Pure virtualized, you using xen pv or virtio? Thanks, Peng. > > > > But even if we restrict to dom0, have you checked that none of the > > SMCs use buffers? > I haven't found any such instances in the Linux kernel where a buffer is used. > Adding a call filtering like suggested below additions of such functions can be > discovered and adapted for if they would show up later. > > > > Rather than providing a blanket forward, to me it sounds more like you > > want to provide an allowlist of the SMCs. This is more futureproof and > > avoid the risk to expose unsafe SMCs to any domain. > > > > For an example, you can have a look at the EEMI mediator for Xilinx. > > Ack. Do you prefer to see only on SMCCC service level or also on function > level? (a1 register, per description earlier) > > > > Cheers, > > > > Thanks! // John Ernberg
On 31/01/2024 15:32, John Ernberg wrote: > Hi Julien, Hi John, > On 1/31/24 13:22, Julien Grall wrote: >> Hi, >> >> On 31/01/2024 11:50, John Ernberg wrote: >>> When using Linux for dom0 there are a bunch of drivers that need to do >>> SMC >>> SIP calls into the PSCI provider to enable certain hardware bits like the >>> watchdog. >> >> Do you know which protocol this is under the hood. Is this SCMI? > > I think I confused myself here when I wrote the commit log. > > The EL3 code in our case is ATF, and it does not appear to be SCMI, nor > PSCI. The register usage of these SMC SIP calls are as follows: > a0 - service > a1 - function > a2-a7 - args > > In ATF the handler is declared as a runtime service. > > Would the appropriate commmit message here be something along the lines > of below? > """ > When using Linux for dom0 there are a bunch of drivers that need to do SMC > SIP calls into the firmware to enable certain hardware bits like the > watchdog. > """ It reads better thanks. [...] >> But even if we restrict to dom0, have you checked that none of the SMCs >> use buffers? > I haven't found any such instances in the Linux kernel where a buffer is > used. Adding a call filtering like suggested below additions of such > functions can be discovered and adapted for if they would show up later. >> >> Rather than providing a blanket forward, to me it sounds more like you >> want to provide an allowlist of the SMCs. This is more futureproof and >> avoid the risk to expose unsafe SMCs to any domain. >> >> For an example, you can have a look at the EEMI mediator for Xilinx. > > Ack. Do you prefer to see only on SMCCC service level or also on > function level? (a1 register, per description earlier) I am not sure. It will depend on whether it is correct to expose *all* the functions within a service level and they have the same format. If you can't guarantee that, then you will most likely need to allowlist at the function level. Also, do you have a spec in hand that would help to understand which service/function is implemented via those SMCs? Cheers,
Hi Peng, On 2/1/24 05:10, Peng Fan wrote: >> Cc: Jonas Blixt <jonas.blixt@actia.se>; xen-devel@lists.xenproject.org >> Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue >> >> Hi Julien, >> >> On 1/31/24 13:22, Julien Grall wrote: >>> Hi, >>> >>> On 31/01/2024 11:50, John Ernberg wrote: >>>> When using Linux for dom0 there are a bunch of drivers that need to >>>> do SMC SIP calls into the PSCI provider to enable certain hardware >>>> bits like the watchdog. >>> >>> Do you know which protocol this is under the hood. Is this SCMI? >> >> I think I confused myself here when I wrote the commit log. >> >> The EL3 code in our case is ATF, and it does not appear to be SCMI, nor PSCI. >> The register usage of these SMC SIP calls are as follows: >> a0 - service >> a1 - function >> a2-a7 - args >> >> In ATF the handler is declared as a runtime service. >> >> Would the appropriate commmit message here be something along the lines >> of below? >> """ >> When using Linux for dom0 there are a bunch of drivers that need to do >> SMC >> SIP calls into the firmware to enable certain hardware bits like the watchdog. >> """ >>> >>>> >>>> Provide a basic platform glue that implements the needed SMC forwarding. >>>> >>>> Signed-off-by: John Ernberg <john.ernberg@actia.se> >>>> --- >>>> NOTE: This is based on code found in NXP Xen tree located here: >>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit >>>> hub.com%2Fnxp-imx%2Fimx-xen%2Fblob%2Flf- >> 5.10.y_4.13%2Fxen%2Farch%2Far >>>> >> m%2Fplatforms%2Fimx8qm.c&data=05%7C02%7Cpeng.fan%40nxp.com%7C >> 573b599a >>>> >> 4b4143ceca1d08dc2271e5be%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C >> 0%7C0%7 >>>> >> C638423119777601548%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA >> wMDAiLCJQI >>>> >> joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=ZO >> 0TXjL6 >>>> g0W7TIZo8x8lTNBXEZW%2BDNcLPndWlEf5D2A%3D&reserved=0 >>> >>> Anything after --- will be removed while applied to the three. I think >>> this NOTE should be written down in the commit message. >> >> Ack. >>> >>> You also possibly want a signed-off-by from Peng as this is his code. >> >> @Peng: May I add a sign-off from you? > > Yeah. You could add my sign off. Great, thanks! > >>> >>>> >>>> xen/arch/arm/platforms/Makefile | 1 + >>>> xen/arch/arm/platforms/imx8qm.c | 65 >>>> +++++++++++++++++++++++++++++++++ >>>> 2 files changed, 66 insertions(+) >>>> create mode 100644 xen/arch/arm/platforms/imx8qm.c >>>> >>>> diff --git a/xen/arch/arm/platforms/Makefile >>>> b/xen/arch/arm/platforms/Makefile index 8632f4115f..bec6e55d1f >> 100644 >>>> --- a/xen/arch/arm/platforms/Makefile >>>> +++ b/xen/arch/arm/platforms/Makefile >>>> @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o >>>> obj-$(CONFIG_ALL64_PLAT) += thunderx.o >>>> obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o >>>> obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o >>>> +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o >>>> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o >>>> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o diff --git >>>> a/xen/arch/arm/platforms/imx8qm.c >> b/xen/arch/arm/platforms/imx8qm.c >>>> new file mode 100644 index 0000000000..a9cd9c3615 >>>> --- /dev/null >>>> +++ b/xen/arch/arm/platforms/imx8qm.c >>>> @@ -0,0 +1,65 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >>>> +/* >>>> + * xen/arch/arm/platforms/imx8qm.c >>>> + * >>>> + * i.MX 8QM setup >>>> + * >>>> + * Copyright (c) 2016 Freescale Inc. >>>> + * Copyright 2018-2019 NXP >>>> + * >>>> + * >>>> + * Peng Fan <peng.fan@nxp.com> >>>> + */ >>>> + >>>> +#include <asm/platform.h> >>>> +#include <asm/smccc.h> >>>> + >>>> +static const char * const imx8qm_dt_compat[] __initconst = { >>>> + "fsl,imx8qm", >>>> + "fsl,imx8qxp", >>>> + NULL >>>> +}; >>>> + >>>> +static bool imx8qm_smc(struct cpu_user_regs *regs) { >>> >>> Your implementation below will not only forward SMC for dom0 but also >>> for any non-trusted domains. Have you investigated that all the SIP >>> calls are safe to be called by anyone? >> >> We use pure virtualized domUs, so we do not expect any calls to this SMC >> interface from the guest. I'll limit it to dom0. > > Would you mind to share what features are supported in your DomU? > > Pure virtualized, you using xen pv or virtio? Not at all. We're forwarding block and networking (and additionally usbip over networking for USB sharing), via the Xen PV drivers. > > Thanks, > Peng. > Thanks! // John Ernberg
On 2/1/24 13:20, Julien Grall wrote: > > > On 31/01/2024 15:32, John Ernberg wrote: >> Hi Julien, > > Hi John, > >> On 1/31/24 13:22, Julien Grall wrote: >>> Hi, >>> >>> On 31/01/2024 11:50, John Ernberg wrote: >>>> When using Linux for dom0 there are a bunch of drivers that need to do >>>> SMC >>>> SIP calls into the PSCI provider to enable certain hardware bits >>>> like the >>>> watchdog. >>> >>> Do you know which protocol this is under the hood. Is this SCMI? >> >> I think I confused myself here when I wrote the commit log. >> >> The EL3 code in our case is ATF, and it does not appear to be SCMI, nor >> PSCI. The register usage of these SMC SIP calls are as follows: >> a0 - service >> a1 - function >> a2-a7 - args >> >> In ATF the handler is declared as a runtime service. >> >> Would the appropriate commmit message here be something along the lines >> of below? >> """ >> When using Linux for dom0 there are a bunch of drivers that need to >> do SMC >> SIP calls into the firmware to enable certain hardware bits like the >> watchdog. >> """ > > It reads better thanks. > > [...] > >>> But even if we restrict to dom0, have you checked that none of the SMCs >>> use buffers? >> I haven't found any such instances in the Linux kernel where a buffer is >> used. Adding a call filtering like suggested below additions of such >> functions can be discovered and adapted for if they would show up later. >>> >>> Rather than providing a blanket forward, to me it sounds more like you >>> want to provide an allowlist of the SMCs. This is more futureproof and >>> avoid the risk to expose unsafe SMCs to any domain. >>> >>> For an example, you can have a look at the EEMI mediator for Xilinx. >> >> Ack. Do you prefer to see only on SMCCC service level or also on >> function level? (a1 register, per description earlier) > > I am not sure. It will depend on whether it is correct to expose *all* > the functions within a service level and they have the same format. > > If you can't guarantee that, then you will most likely need to allowlist > at the function level. > > Also, do you have a spec in hand that would help to understand which > service/function is implemented via those SMCs? I don't have the spec unfortunately, but I will add a filter on both service and function for V2 and we'll take it from there. > > Cheers, > Thanks! // John Ernberg
On 01/02/2024 16:17, John Ernberg wrote: > On 2/1/24 13:20, Julien Grall wrote: >> >> >> On 31/01/2024 15:32, John Ernberg wrote: >>> Hi Julien, >> >> Hi John, >> >>> On 1/31/24 13:22, Julien Grall wrote: >>>> Hi, >>>> >>>> On 31/01/2024 11:50, John Ernberg wrote: >>>>> When using Linux for dom0 there are a bunch of drivers that need to do >>>>> SMC >>>>> SIP calls into the PSCI provider to enable certain hardware bits >>>>> like the >>>>> watchdog. >>>> >>>> Do you know which protocol this is under the hood. Is this SCMI? >>> >>> I think I confused myself here when I wrote the commit log. >>> >>> The EL3 code in our case is ATF, and it does not appear to be SCMI, nor >>> PSCI. The register usage of these SMC SIP calls are as follows: >>> a0 - service >>> a1 - function >>> a2-a7 - args >>> >>> In ATF the handler is declared as a runtime service. >>> >>> Would the appropriate commmit message here be something along the lines >>> of below? >>> """ >>> When using Linux for dom0 there are a bunch of drivers that need to >>> do SMC >>> SIP calls into the firmware to enable certain hardware bits like the >>> watchdog. >>> """ >> >> It reads better thanks. >> >> [...] >> >>>> But even if we restrict to dom0, have you checked that none of the SMCs >>>> use buffers? >>> I haven't found any such instances in the Linux kernel where a buffer is >>> used. Adding a call filtering like suggested below additions of such >>> functions can be discovered and adapted for if they would show up later. >>>> >>>> Rather than providing a blanket forward, to me it sounds more like you >>>> want to provide an allowlist of the SMCs. This is more futureproof and >>>> avoid the risk to expose unsafe SMCs to any domain. >>>> >>>> For an example, you can have a look at the EEMI mediator for Xilinx. >>> >>> Ack. Do you prefer to see only on SMCCC service level or also on >>> function level? (a1 register, per description earlier) >> >> I am not sure. It will depend on whether it is correct to expose *all* >> the functions within a service level and they have the same format. >> >> If you can't guarantee that, then you will most likely need to allowlist >> at the function level. >> >> Also, do you have a spec in hand that would help to understand which >> service/function is implemented via those SMCs? > > I don't have the spec unfortunately, but I will add a filter on both > service and function for V2 and we'll take it from there. @Peng, do you have any specification you could share? How stable is the interface? Cheers,
On Fri, 2 Feb 2024, Julien Grall wrote: > On 01/02/2024 16:17, John Ernberg wrote: > > On 2/1/24 13:20, Julien Grall wrote: > > > > > > > > > On 31/01/2024 15:32, John Ernberg wrote: > > > > Hi Julien, > > > > > > Hi John, > > > > > > > On 1/31/24 13:22, Julien Grall wrote: > > > > > Hi, > > > > > > > > > > On 31/01/2024 11:50, John Ernberg wrote: > > > > > > When using Linux for dom0 there are a bunch of drivers that need to > > > > > > do > > > > > > SMC > > > > > > SIP calls into the PSCI provider to enable certain hardware bits > > > > > > like the > > > > > > watchdog. > > > > > > > > > > Do you know which protocol this is under the hood. Is this SCMI? > > > > > > > > I think I confused myself here when I wrote the commit log. > > > > > > > > The EL3 code in our case is ATF, and it does not appear to be SCMI, nor > > > > PSCI. The register usage of these SMC SIP calls are as follows: > > > > a0 - service > > > > a1 - function > > > > a2-a7 - args > > > > > > > > In ATF the handler is declared as a runtime service. > > > > > > > > Would the appropriate commmit message here be something along the lines > > > > of below? > > > > """ > > > > When using Linux for dom0 there are a bunch of drivers that need to > > > > do SMC > > > > SIP calls into the firmware to enable certain hardware bits like the > > > > watchdog. > > > > """ > > > > > > It reads better thanks. > > > > > > [...] > > > > > > > > But even if we restrict to dom0, have you checked that none of the > > > > > SMCs > > > > > use buffers? > > > > I haven't found any such instances in the Linux kernel where a buffer is > > > > used. Adding a call filtering like suggested below additions of such > > > > functions can be discovered and adapted for if they would show up later. > > > > > > > > > > Rather than providing a blanket forward, to me it sounds more like you > > > > > want to provide an allowlist of the SMCs. This is more futureproof and > > > > > avoid the risk to expose unsafe SMCs to any domain. > > > > > > > > > > For an example, you can have a look at the EEMI mediator for Xilinx. > > > > > > > > Ack. Do you prefer to see only on SMCCC service level or also on > > > > function level? (a1 register, per description earlier) > > > > > > I am not sure. It will depend on whether it is correct to expose *all* > > > the functions within a service level and they have the same format. > > > > > > If you can't guarantee that, then you will most likely need to allowlist > > > at the function level. > > > > > > Also, do you have a spec in hand that would help to understand which > > > service/function is implemented via those SMCs? > > > > I don't have the spec unfortunately, but I will add a filter on both > > service and function for V2 and we'll take it from there. > > @Peng, do you have any specification you could share? How stable is the > interface? Just to add some context to make the reason for the question clearer, if we have a specification we could check the patch for correctness. Without it, it is difficult to know if it is doing the right thing. The other aspect is about expectation of forward and backward compatibility. Can we guarantee that the next version of Xen and the one after it will still work against this interface? If not, can we check for the version of the interface before continuing? If not, can we at least document that the interface is only known-to-work with specific firmware versions? This is basically just to provide the right expectations to users and ideally to prevent a future version of Xen to break on boot silently without information. If we don't have a spec and we don't know if the interface is stable, I think we should try to detect the version of the interface and print a warning in Xen if it not a known version.
> -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2024年2月2日 17:20 > To: John Ernberg <john.ernberg@actia.se>; Stefano Stabellini > <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; > Michal Orzel <michal.orzel@amd.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com>; Peng Fan <peng.fan@nxp.com> > Cc: Jonas Blixt <jonas.blixt@actia.se>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue > > On 01/02/2024 16:17, John Ernberg wrote: > > On 2/1/24 13:20, Julien Grall wrote: > >> > >> > >> On 31/01/2024 15:32, John Ernberg wrote: > >>> Hi Julien, > >> > >> Hi John, > >> > >>> On 1/31/24 13:22, Julien Grall wrote: > >>>> Hi, > >>>> > >>>> On 31/01/2024 11:50, John Ernberg wrote: > >>>>> When using Linux for dom0 there are a bunch of drivers that need > >>>>> to do SMC SIP calls into the PSCI provider to enable certain > >>>>> hardware bits like the watchdog. > >>>> > >>>> Do you know which protocol this is under the hood. Is this SCMI? > >>> > >>> I think I confused myself here when I wrote the commit log. > >>> > >>> The EL3 code in our case is ATF, and it does not appear to be SCMI, > >>> nor PSCI. The register usage of these SMC SIP calls are as follows: > >>> a0 - service > >>> a1 - function > >>> a2-a7 - args > >>> > >>> In ATF the handler is declared as a runtime service. > >>> > >>> Would the appropriate commmit message here be something along the > >>> lines of below? > >>> """ > >>> When using Linux for dom0 there are a bunch of drivers that need to > >>> do SMC SIP calls into the firmware to enable certain hardware bits > >>> like the watchdog. > >>> """ > >> > >> It reads better thanks. > >> > >> [...] > >> > >>>> But even if we restrict to dom0, have you checked that none of the > >>>> SMCs use buffers? > >>> I haven't found any such instances in the Linux kernel where a > >>> buffer is used. Adding a call filtering like suggested below > >>> additions of such functions can be discovered and adapted for if they > would show up later. > >>>> > >>>> Rather than providing a blanket forward, to me it sounds more like > >>>> you want to provide an allowlist of the SMCs. This is more > >>>> futureproof and avoid the risk to expose unsafe SMCs to any domain. > >>>> > >>>> For an example, you can have a look at the EEMI mediator for Xilinx. > >>> > >>> Ack. Do you prefer to see only on SMCCC service level or also on > >>> function level? (a1 register, per description earlier) > >> > >> I am not sure. It will depend on whether it is correct to expose > >> *all* the functions within a service level and they have the same format. > >> > >> If you can't guarantee that, then you will most likely need to > >> allowlist at the function level. > >> > >> Also, do you have a spec in hand that would help to understand which > >> service/function is implemented via those SMCs? > > > > I don't have the spec unfortunately, but I will add a filter on both > > service and function for V2 and we'll take it from there. > > @Peng, do you have any specification you could share? How stable is the > interface? No specification, the use is IMX_SIP_X in linux kernel source. Such as IMX_SIP_RTC, IMX_SIP_TIMER It is stable and no change, we only add new SIP macro if needed and no change the meaning or reuse old SIP. Regards, Peng. > > Cheers, > > -- > Julien Grall
Hi, On 04/02/2024 09:40, Peng Fan wrote: > > >> -----Original Message----- >> From: Julien Grall <julien@xen.org> >> Sent: 2024年2月2日 17:20 >> To: John Ernberg <john.ernberg@actia.se>; Stefano Stabellini >> <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; >> Michal Orzel <michal.orzel@amd.com>; Volodymyr Babchuk >> <Volodymyr_Babchuk@epam.com>; Peng Fan <peng.fan@nxp.com> >> Cc: Jonas Blixt <jonas.blixt@actia.se>; xen-devel@lists.xenproject.org >> Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue >> >> On 01/02/2024 16:17, John Ernberg wrote: >>> On 2/1/24 13:20, Julien Grall wrote: >>>> >>>> >>>> On 31/01/2024 15:32, John Ernberg wrote: >>>>> Hi Julien, >>>> >>>> Hi John, >>>> >>>>> On 1/31/24 13:22, Julien Grall wrote: >>>>>> Hi, >>>>>> >>>>>> On 31/01/2024 11:50, John Ernberg wrote: >>>>>>> When using Linux for dom0 there are a bunch of drivers that need >>>>>>> to do SMC SIP calls into the PSCI provider to enable certain >>>>>>> hardware bits like the watchdog. >>>>>> >>>>>> Do you know which protocol this is under the hood. Is this SCMI? >>>>> >>>>> I think I confused myself here when I wrote the commit log. >>>>> >>>>> The EL3 code in our case is ATF, and it does not appear to be SCMI, >>>>> nor PSCI. The register usage of these SMC SIP calls are as follows: >>>>> a0 - service >>>>> a1 - function >>>>> a2-a7 - args >>>>> >>>>> In ATF the handler is declared as a runtime service. >>>>> >>>>> Would the appropriate commmit message here be something along the >>>>> lines of below? >>>>> """ >>>>> When using Linux for dom0 there are a bunch of drivers that need to >>>>> do SMC SIP calls into the firmware to enable certain hardware bits >>>>> like the watchdog. >>>>> """ >>>> >>>> It reads better thanks. >>>> >>>> [...] >>>> >>>>>> But even if we restrict to dom0, have you checked that none of the >>>>>> SMCs use buffers? >>>>> I haven't found any such instances in the Linux kernel where a >>>>> buffer is used. Adding a call filtering like suggested below >>>>> additions of such functions can be discovered and adapted for if they >> would show up later. >>>>>> >>>>>> Rather than providing a blanket forward, to me it sounds more like >>>>>> you want to provide an allowlist of the SMCs. This is more >>>>>> futureproof and avoid the risk to expose unsafe SMCs to any domain. >>>>>> >>>>>> For an example, you can have a look at the EEMI mediator for Xilinx. >>>>> >>>>> Ack. Do you prefer to see only on SMCCC service level or also on >>>>> function level? (a1 register, per description earlier) >>>> >>>> I am not sure. It will depend on whether it is correct to expose >>>> *all* the functions within a service level and they have the same format. >>>> >>>> If you can't guarantee that, then you will most likely need to >>>> allowlist at the function level. >>>> >>>> Also, do you have a spec in hand that would help to understand which >>>> service/function is implemented via those SMCs? >>> >>> I don't have the spec unfortunately, but I will add a filter on both >>> service and function for V2 and we'll take it from there. >> >> @Peng, do you have any specification you could share? How stable is the >> interface? > > No specification, the use is IMX_SIP_X in linux kernel source. > > Such as IMX_SIP_RTC, IMX_SIP_TIMER > > It is stable and no change, we only add new SIP macro if needed > and no change the meaning or reuse old SIP. Thanks for the answer. It is a bit unfortunate there are no specification, but at least they are stable. I have searched IMX_SIP in Linux, there doesn't seem many so we could allowlist them (see more below). Do you know if there are more necessary that are not yet upstreamed in Linux? Looking through the list, there are some that probably want a bit more discussion on whether we want to expose them: * IMX_SIP_CPUFREQ: Right now, dom0 is not aware of the full system. So it may take wrong decision. * IMX_SIP_DDR_DVFS: Some operation seems to take the number of online CPUs. Dom0 doesn't know that * IMX_SIP_TIMER_*: This seems to be related to the watchdog. Shouldn't dom0 rely on the watchdog provided by Xen instead? So those call will be used by Xen. Also, what happen if we don't expose those SMC to dom0? Cheers,
Hi Julien, Apologies for the delay, I was pulled away for a bit. On 2/5/24 11:13, Julien Grall wrote: > Hi, > > On 04/02/2024 09:40, Peng Fan wrote: >> >> >>> -----Original Message----- >>> From: Julien Grall <julien@xen.org> >>> Sent: 2024年2月2日 17:20 >>> To: John Ernberg <john.ernberg@actia.se>; Stefano Stabellini >>> <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; >>> Michal Orzel <michal.orzel@amd.com>; Volodymyr Babchuk >>> <Volodymyr_Babchuk@epam.com>; Peng Fan <peng.fan@nxp.com> >>> Cc: Jonas Blixt <jonas.blixt@actia.se>; xen-devel@lists.xenproject.org >>> Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue >>> >>> On 01/02/2024 16:17, John Ernberg wrote: >>>> On 2/1/24 13:20, Julien Grall wrote: >>>>> >>>>> >>>>> On 31/01/2024 15:32, John Ernberg wrote: >>>>>> Hi Julien, >>>>> >>>>> Hi John, >>>>> >>>>>> On 1/31/24 13:22, Julien Grall wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On 31/01/2024 11:50, John Ernberg wrote: [ ... ] >>>>> >>>>>>> But even if we restrict to dom0, have you checked that none of the >>>>>>> SMCs use buffers? >>>>>> I haven't found any such instances in the Linux kernel where a >>>>>> buffer is used. Adding a call filtering like suggested below >>>>>> additions of such functions can be discovered and adapted for if they >>> would show up later. >>>>>>> >>>>>>> Rather than providing a blanket forward, to me it sounds more like >>>>>>> you want to provide an allowlist of the SMCs. This is more >>>>>>> futureproof and avoid the risk to expose unsafe SMCs to any domain. >>>>>>> >>>>>>> For an example, you can have a look at the EEMI mediator for Xilinx. >>>>>> >>>>>> Ack. Do you prefer to see only on SMCCC service level or also on >>>>>> function level? (a1 register, per description earlier) >>>>> >>>>> I am not sure. It will depend on whether it is correct to expose >>>>> *all* the functions within a service level and they have the same >>>>> format. >>>>> >>>>> If you can't guarantee that, then you will most likely need to >>>>> allowlist at the function level. >>>>> >>>>> Also, do you have a spec in hand that would help to understand which >>>>> service/function is implemented via those SMCs? >>>> >>>> I don't have the spec unfortunately, but I will add a filter on both >>>> service and function for V2 and we'll take it from there. >>> >>> @Peng, do you have any specification you could share? How stable is the >>> interface? >> >> No specification, the use is IMX_SIP_X in linux kernel source. >> >> Such as IMX_SIP_RTC, IMX_SIP_TIMER >> >> It is stable and no change, we only add new SIP macro if needed >> and no change the meaning or reuse old SIP. > > Thanks for the answer. It is a bit unfortunate there are no > specification, but at least they are stable. > > I have searched IMX_SIP in Linux, there doesn't seem many so we could > allowlist them (see more below). Do you know if there are more necessary > that are not yet upstreamed in Linux? I took a dive into both upstream kernel and the vendor tree and found the following list and for which SoCs they are applicable (Please correct me if you can Peng) 0x82000006 IMX_SIP_BUSFREQ_CHANGE [unsure, probably not imx8] 0xC2000000 IMX_SIP_GPC [only imx8m series] 0xC2000001 IMX_SIP_CPUFREQ [only imx8q{x,m} series] 0xC2000002 IMX_SIP_SRTC / IMX_SIP_TIMER [only imx8q{x,m} series] 0xC2000004 IMX_SIP_DDR_DVFS [only imx8m series] 0xC2000005 IMX_SIP_RPROC / IMX_SIP_SRC [only imx8m series] 0xC2000006 IMX_SIP_GET_SOC_INFO [only imx8m series] 0xC2000008 IMX_SIP_NOC [only imx8m series] 0xC2000009 IMX_SIP_WAKEUP_SRC [only imx8q{x,m} series] 0xC200000B IMX_SIP_OTP_WRITE [only imx8q{x,m} series] > > > Looking through the list, there are some that probably want a bit more > discussion on whether we want to expose them: > * IMX_SIP_CPUFREQ: Right now, dom0 is not aware of the full system. > So it may take wrong decision. The cpufreq function operates on the cluster level, it performs no error checking so blocking it will be invisible to Linux. I don't know yet what kind of impact that could have. Looking for hints about cpufreq in Xen for ARM I found [1] and [2]. In our current setup we do not use cpufreq and it is stable enough for our usecase. From my point of view we can block it for now, and when cpufreq functionality gets picked up again, we can revisit. > * IMX_SIP_DDR_DVFS: Some operation seems to take the number of online > CPUs. Dom0 doesn't know that The iMX8Q{X,M} series comes with a system controller called the SCU, running in a small M-core. This controller takes care of all the DDR bits, so we do not need to care for these SoCs. > * IMX_SIP_TIMER_*: This seems to be related to the watchdog. > Shouldn't dom0 rely on the watchdog provided by Xen instead? So those > call will be used by Xen. That is indeed a watchdog SIP, and also for setting the SoC internal RTC if it is being used. I looked around if there was previous discussion and only really found [3]. Is the xen/xen/include/watchdog.h header meant to be for this kind of watchdog support or is that more for the VM watchdog? Looking at the x86 ACPI NMI watchdog it seems like the former, but I have never worked with x86 nor ACPI. Currently forwarding it to Dom0 has not caused any watchdog resets with our watchdog timeout settings, our specific Dom0 setup and VM count. For the remaining bits: The wakeup src is not in the upstream kernel yet and related to system resume from suspend which isn't supported in Xen on ARM yet - so this we can block safely. The OTP write is fuse programming in the SoC fuse banks, and should probably be allowed but reserved for Dom0, as this can set fuses that affects the SoC boot. > > Also, what happen if we don't expose those SMC to dom0? > > Cheers, > [1]: https://lore.kernel.org/xen-devel/1510247421-24094-1-git-send-email-olekstysh@gmail.com/ [2]: https://www.slideshare.net/xen_com_mgr/xpdds18-cpufreq-in-xen-on-arm-oleksandr-tyshchenko-epam-systems [3]: https://xen-users.narkive.com/ISXnlmB0/watchdog-support-in-xen Thanks! // John Ernberg
Hi Julien, On 2/9/24 14:14, John Ernberg wrote: > Hi Julien, > > Apologies for the delay, I was pulled away for a bit. > > On 2/5/24 11:13, Julien Grall wrote: >> Hi, >> >> On 04/02/2024 09:40, Peng Fan wrote: >>> >>> >>>> -----Original Message----- >>>> From: Julien Grall <julien@xen.org> >>>> Sent: 2024年2月2日 17:20 >>>> To: John Ernberg <john.ernberg@actia.se>; Stefano Stabellini >>>> <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; >>>> Michal Orzel <michal.orzel@amd.com>; Volodymyr Babchuk >>>> <Volodymyr_Babchuk@epam.com>; Peng Fan <peng.fan@nxp.com> >>>> Cc: Jonas Blixt <jonas.blixt@actia.se>; xen-devel@lists.xenproject.org >>>> Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue >>>> >>>> On 01/02/2024 16:17, John Ernberg wrote: >>>>> On 2/1/24 13:20, Julien Grall wrote: >>>>>> >>>>>> >>>>>> On 31/01/2024 15:32, John Ernberg wrote: >>>>>>> Hi Julien, >>>>>> >>>>>> Hi John, >>>>>> >>>>>>> On 1/31/24 13:22, Julien Grall wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 31/01/2024 11:50, John Ernberg wrote: > [ ... ] >>>>>> >>>>>>>> But even if we restrict to dom0, have you checked that none of the >>>>>>>> SMCs use buffers? >>>>>>> I haven't found any such instances in the Linux kernel where a >>>>>>> buffer is used. Adding a call filtering like suggested below >>>>>>> additions of such functions can be discovered and adapted for if >>>>>>> they >>>> would show up later. >>>>>>>> >>>>>>>> Rather than providing a blanket forward, to me it sounds more like >>>>>>>> you want to provide an allowlist of the SMCs. This is more >>>>>>>> futureproof and avoid the risk to expose unsafe SMCs to any domain. >>>>>>>> >>>>>>>> For an example, you can have a look at the EEMI mediator for >>>>>>>> Xilinx. >>>>>>> >>>>>>> Ack. Do you prefer to see only on SMCCC service level or also on >>>>>>> function level? (a1 register, per description earlier) >>>>>> >>>>>> I am not sure. It will depend on whether it is correct to expose >>>>>> *all* the functions within a service level and they have the same >>>>>> format. >>>>>> >>>>>> If you can't guarantee that, then you will most likely need to >>>>>> allowlist at the function level. >>>>>> >>>>>> Also, do you have a spec in hand that would help to understand which >>>>>> service/function is implemented via those SMCs? >>>>> >>>>> I don't have the spec unfortunately, but I will add a filter on both >>>>> service and function for V2 and we'll take it from there. >>>> >>>> @Peng, do you have any specification you could share? How stable is the >>>> interface? >>> >>> No specification, the use is IMX_SIP_X in linux kernel source. >>> >>> Such as IMX_SIP_RTC, IMX_SIP_TIMER >>> >>> It is stable and no change, we only add new SIP macro if needed >>> and no change the meaning or reuse old SIP. >> >> Thanks for the answer. It is a bit unfortunate there are no >> specification, but at least they are stable. >> >> I have searched IMX_SIP in Linux, there doesn't seem many so we could >> allowlist them (see more below). Do you know if there are more >> necessary that are not yet upstreamed in Linux? > > I took a dive into both upstream kernel and the vendor tree and found > the following list and for which SoCs they are applicable (Please > correct me if you can Peng) > > 0x82000006 IMX_SIP_BUSFREQ_CHANGE [unsure, probably not imx8] > 0xC2000000 IMX_SIP_GPC [only imx8m series] > 0xC2000001 IMX_SIP_CPUFREQ [only imx8q{x,m} series] > 0xC2000002 IMX_SIP_SRTC / IMX_SIP_TIMER [only imx8q{x,m} series] > 0xC2000004 IMX_SIP_DDR_DVFS [only imx8m series] > 0xC2000005 IMX_SIP_RPROC / IMX_SIP_SRC [only imx8m series] > 0xC2000006 IMX_SIP_GET_SOC_INFO [only imx8m series] > 0xC2000008 IMX_SIP_NOC [only imx8m series] > 0xC2000009 IMX_SIP_WAKEUP_SRC [only imx8q{x,m} series] > 0xC200000B IMX_SIP_OTP_WRITE [only imx8q{x,m} series] >> >> [ ... ] > >> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >> call will be used by Xen. > > That is indeed a watchdog SIP, and also for setting the SoC internal RTC > if it is being used. > > I looked around if there was previous discussion and only really found [3]. > Is the xen/xen/include/watchdog.h header meant to be for this kind of > watchdog support or is that more for the VM watchdog? Looking at the x86 > ACPI NMI watchdog it seems like the former, but I have never worked with > x86 nor ACPI. > > Currently forwarding it to Dom0 has not caused any watchdog resets with > our watchdog timeout settings, our specific Dom0 setup and VM count. > > For the remaining bits: > > The wakeup src is not in the upstream kernel yet and related to system > resume from suspend which isn't supported in Xen on ARM yet - so this we > can block safely. > > The OTP write is fuse programming in the SoC fuse banks, and should > probably be allowed but reserved for Dom0, as this can set fuses that > affects the SoC boot. > >> >> Also, what happen if we don't expose those SMC to dom0? >> >> Cheers, >> > > [1]: > https://lore.kernel.org/xen-devel/1510247421-24094-1-git-send-email-olekstysh@gmail.com/ > [2]: > https://www.slideshare.net/xen_com_mgr/xpdds18-cpufreq-in-xen-on-arm-oleksandr-tyshchenko-epam-systems > [3]: https://xen-users.narkive.com/ISXnlmB0/watchdog-support-in-xen > > Thanks! // John Ernberg Ping on the watchdog discussion bits. Thanks! // John Ernberg
Hi John, > Ping on the watchdog discussion bits. Sorry for the late reply. On 06/03/2024 13:13, John Ernberg wrote: > On 2/9/24 14:14, John Ernberg wrote: >> >>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>> call will be used by Xen. >> >> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >> if it is being used. >> >> I looked around if there was previous discussion and only really found [3]. >> Is the xen/xen/include/watchdog.h header meant to be for this kind of >> watchdog support or is that more for the VM watchdog? Looking at the x86 >> ACPI NMI watchdog it seems like the former, but I have never worked with >> x86 nor ACPI. include/watchdog.h contains helper to configure the watchdog for Xen. We also have per-VM watchdog and this is configured by the hypercall SCHEDOP_watchdog. >> >> Currently forwarding it to Dom0 has not caused any watchdog resets with >> our watchdog timeout settings, our specific Dom0 setup and VM count. IIUC, the SMC API for the watchdog would be similar to the ACPI NMI watchdog. So I think it would make more sense if this is not exposed to dom0 (even if Xen is doing nothing with it). Can you try to hide the SMCs and check if dom0 still behave properly? Cheers,
Hi Julien, On 3/7/24 00:07, Julien Grall wrote: > Hi John, > > > Ping on the watchdog discussion bits. > > Sorry for the late reply. > > On 06/03/2024 13:13, John Ernberg wrote: >> On 2/9/24 14:14, John Ernberg wrote: >>> >>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>> call will be used by Xen. >>> >>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>> if it is being used. >>> >>> I looked around if there was previous discussion and only really >>> found [3]. >>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>> ACPI NMI watchdog it seems like the former, but I have never worked with >>> x86 nor ACPI. > > include/watchdog.h contains helper to configure the watchdog for Xen. We > also have per-VM watchdog and this is configured by the hypercall > SCHEDOP_watchdog. > >>> >>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>> our watchdog timeout settings, our specific Dom0 setup and VM count. > > IIUC, the SMC API for the watchdog would be similar to the ACPI NMI > watchdog. So I think it would make more sense if this is not exposed to > dom0 (even if Xen is doing nothing with it). > > Can you try to hide the SMCs and check if dom0 still behave properly? > > Cheers, > This SMC manages a hardware watchdog, if it's not pinged within a specific interval the entire board resets. If I block the SMCs the watchdog driver in Dom0 will fail to ping the watchdog, triggering a board reset because the system looks to have become unresponsive. The reason this patch set started is because we couldn't ping the watchdog when running with Xen. In our specific system the bootloader enables the watchdog as early as possible so that we can get watchdog protection for as much of the boot as we possibly can. So, if we are to block the SMC from Dom0, then Xen needs to take over the pinging. It could be implemented similarly to the NMI watchdog, except that the system will reset if the ping is missed rather than backtrace. It would also mean that Xen will get a whole watchdog driver-category due to the watchdog being vendor and sometimes even SoC specific when it comes to Arm. My understanding of the domain watchdog code is that today the domain needs to call SCHEDOP_watchdog at least once to start the watchdog timer. Since watchdog protection through the whole boot process is desirable we'd need some core changes, such as an option to start the domain watchdog on init. It's quite a big change to make, while I am not against doing it if it makes sense, I now wonder if Xen should manage hardware watchdogs. Looking at the domain watchdog code it looks like if a domain does not get enough execution time, the watchdog will not be pinged enough and the guest will be reset. So either watchdog approach requires Dom0 to get execution time. Dom0 also needs to service all the PV backends it's responsible for. I'm not sure it's valuable to add another layer of watchdog for this scenario as the end result (checking that the entire system works) is achieved without it as well. So, before I try to find the time to make a proposal for moving the hardware watchdog bit to Xen, do we really want it? Thanks! // John Ernberg
Hi John, Thank you for the reply. On 08/03/2024 13:40, John Ernberg wrote: > On 3/7/24 00:07, Julien Grall wrote: >> > Ping on the watchdog discussion bits. >> >> Sorry for the late reply. >> >> On 06/03/2024 13:13, John Ernberg wrote: >>> On 2/9/24 14:14, John Ernberg wrote: >>>> >>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>> call will be used by Xen. >>>> >>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>> if it is being used. >>>> >>>> I looked around if there was previous discussion and only really >>>> found [3]. >>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>> x86 nor ACPI. >> >> include/watchdog.h contains helper to configure the watchdog for Xen. We >> also have per-VM watchdog and this is configured by the hypercall >> SCHEDOP_watchdog. >> >>>> >>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >> >> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >> watchdog. So I think it would make more sense if this is not exposed to >> dom0 (even if Xen is doing nothing with it). >> >> Can you try to hide the SMCs and check if dom0 still behave properly? >> >> Cheers, >> > > This SMC manages a hardware watchdog, if it's not pinged within a > specific interval the entire board resets. Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? > > If I block the SMCs the watchdog driver in Dom0 will fail to ping the > watchdog, triggering a board reset because the system looks to have > become unresponsive. The reason this patch set started is because we > couldn't ping the watchdog when running with Xen. > > In our specific system the bootloader enables the watchdog as early as > possible so that we can get watchdog protection for as much of the boot > as we possibly can. > > So, if we are to block the SMC from Dom0, then Xen needs to take over > the pinging. It could be implemented similarly to the NMI watchdog, > except that the system will reset if the ping is missed rather than > backtrace. > It would also mean that Xen will get a whole watchdog driver-category > due to the watchdog being vendor and sometimes even SoC specific when it > comes to Arm. > > My understanding of the domain watchdog code is that today the domain > needs to call SCHEDOP_watchdog at least once to start the watchdog > timer. Since watchdog protection through the whole boot process is > desirable we'd need some core changes, such as an option to start the > domain watchdog on init. > > It's quite a big change to make For clarification, above you seem to mention two changes: 1) Allow Xen to use the HW watchdog 2) Allow the domain to use the watchdog early I am assuming by big change, you are referring to 2? , while I am not against doing it if it > makes sense, I now wonder if Xen should manage hardware watchdogs. > Looking at the domain watchdog code it looks like if a domain does not > get enough execution time, the watchdog will not be pinged enough and > the guest will be reset. So either watchdog approach requires Dom0 to > get execution time. Dom0 also needs to service all the PV backends it's > responsible for. I'm not sure it's valuable to add another layer of > watchdog for this scenario as the end result (checking that the entire > system works) is achieved without it as well. > > So, before I try to find the time to make a proposal for moving the > hardware watchdog bit to Xen, do we really want it? Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: 1) In true dom0less environment, dom0 would not exist 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. Let see what the other Arm maintainer thinks.
Hi Julien, On 3/8/24 15:04, Julien Grall wrote: > Hi John, > > Thank you for the reply. > > On 08/03/2024 13:40, John Ernberg wrote: >> On 3/7/24 00:07, Julien Grall wrote: >>> > Ping on the watchdog discussion bits. >>> >>> Sorry for the late reply. >>> >>> On 06/03/2024 13:13, John Ernberg wrote: >>>> On 2/9/24 14:14, John Ernberg wrote: >>>>> >>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>> call will be used by Xen. >>>>> >>>>> That is indeed a watchdog SIP, and also for setting the SoC >>>>> internal RTC >>>>> if it is being used. >>>>> >>>>> I looked around if there was previous discussion and only really >>>>> found [3]. >>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>> watchdog support or is that more for the VM watchdog? Looking at >>>>> the x86 >>>>> ACPI NMI watchdog it seems like the former, but I have never worked >>>>> with >>>>> x86 nor ACPI. >>> >>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>> also have per-VM watchdog and this is configured by the hypercall >>> SCHEDOP_watchdog. >>> >>>>> >>>>> Currently forwarding it to Dom0 has not caused any watchdog resets >>>>> with >>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>> >>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>> watchdog. So I think it would make more sense if this is not exposed to >>> dom0 (even if Xen is doing nothing with it). >>> >>> Can you try to hide the SMCs and check if dom0 still behave properly? >>> >>> Cheers, >>> >> >> This SMC manages a hardware watchdog, if it's not pinged within a >> specific interval the entire board resets. > > Do you know what's the default interval? Is it large enough so Xen + > dom0 can boot (at least up to when the watchdog driver is initialized)? > >> >> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >> watchdog, triggering a board reset because the system looks to have >> become unresponsive. The reason this patch set started is because we >> couldn't ping the watchdog when running with Xen. >> >> In our specific system the bootloader enables the watchdog as early as >> possible so that we can get watchdog protection for as much of the boot >> as we possibly can. >> >> So, if we are to block the SMC from Dom0, then Xen needs to take over >> the pinging. It could be implemented similarly to the NMI watchdog, >> except that the system will reset if the ping is missed rather than >> backtrace. >> It would also mean that Xen will get a whole watchdog driver-category >> due to the watchdog being vendor and sometimes even SoC specific when it >> comes to Arm. >> >> My understanding of the domain watchdog code is that today the domain >> needs to call SCHEDOP_watchdog at least once to start the watchdog >> timer. Since watchdog protection through the whole boot process is >> desirable we'd need some core changes, such as an option to start the >> domain watchdog on init. > >> It's quite a big change to make > > For clarification, above you seem to mention two changes: > > 1) Allow Xen to use the HW watchdog > 2) Allow the domain to use the watchdog early > > I am assuming by big change, you are referring to 2? Both of them. I'm expecting the addition of a new driver category (hardware watchdog) to be a decent amount of work as well. > > , while I am not against doing it if it >> makes sense, I now wonder if Xen should manage hardware watchdogs. >> Looking at the domain watchdog code it looks like if a domain does not >> get enough execution time, the watchdog will not be pinged enough and >> the guest will be reset. So either watchdog approach requires Dom0 to >> get execution time. Dom0 also needs to service all the PV backends it's >> responsible for. I'm not sure it's valuable to add another layer of >> watchdog for this scenario as the end result (checking that the entire >> system works) is achieved without it as well. >> >> So, before I try to find the time to make a proposal for moving the >> hardware watchdog bit to Xen, do we really want it? > > Thanks for the details. Given that the watchdog is enabled by the > bootloader, I think we want Xen to drive the watchdog for two reasons: > 1) In true dom0less environment, dom0 would not exist > 2) You are relying on Xen + Dom0 to boot (or at least enough to get > the watchdog working) within the watchdog interval. > > Let see what the other Arm maintainer thinks. > Regards // John Ernberg
Hi, > On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: > > Hi John, > > Thank you for the reply. > > On 08/03/2024 13:40, John Ernberg wrote: >> On 3/7/24 00:07, Julien Grall wrote: >>> > Ping on the watchdog discussion bits. >>> >>> Sorry for the late reply. >>> >>> On 06/03/2024 13:13, John Ernberg wrote: >>>> On 2/9/24 14:14, John Ernberg wrote: >>>>> >>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>> call will be used by Xen. >>>>> >>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>> if it is being used. >>>>> >>>>> I looked around if there was previous discussion and only really >>>>> found [3]. >>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>> x86 nor ACPI. >>> >>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>> also have per-VM watchdog and this is configured by the hypercall >>> SCHEDOP_watchdog. >>> >>>>> >>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>> >>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>> watchdog. So I think it would make more sense if this is not exposed to >>> dom0 (even if Xen is doing nothing with it). >>> >>> Can you try to hide the SMCs and check if dom0 still behave properly? >>> >>> Cheers, >>> >> This SMC manages a hardware watchdog, if it's not pinged within a >> specific interval the entire board resets. > > Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? > >> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >> watchdog, triggering a board reset because the system looks to have >> become unresponsive. The reason this patch set started is because we >> couldn't ping the watchdog when running with Xen. >> In our specific system the bootloader enables the watchdog as early as >> possible so that we can get watchdog protection for as much of the boot >> as we possibly can. >> So, if we are to block the SMC from Dom0, then Xen needs to take over >> the pinging. It could be implemented similarly to the NMI watchdog, >> except that the system will reset if the ping is missed rather than >> backtrace. >> It would also mean that Xen will get a whole watchdog driver-category >> due to the watchdog being vendor and sometimes even SoC specific when it >> comes to Arm. >> My understanding of the domain watchdog code is that today the domain >> needs to call SCHEDOP_watchdog at least once to start the watchdog >> timer. Since watchdog protection through the whole boot process is >> desirable we'd need some core changes, such as an option to start the >> domain watchdog on init. > >> It's quite a big change to make > > For clarification, above you seem to mention two changes: > > 1) Allow Xen to use the HW watchdog > 2) Allow the domain to use the watchdog early > > I am assuming by big change, you are referring to 2? > > , while I am not against doing it if it >> makes sense, I now wonder if Xen should manage hardware watchdogs. >> Looking at the domain watchdog code it looks like if a domain does not >> get enough execution time, the watchdog will not be pinged enough and >> the guest will be reset. So either watchdog approach requires Dom0 to >> get execution time. Dom0 also needs to service all the PV backends it's >> responsible for. I'm not sure it's valuable to add another layer of >> watchdog for this scenario as the end result (checking that the entire >> system works) is achieved without it as well. >> So, before I try to find the time to make a proposal for moving the >> hardware watchdog bit to Xen, do we really want it? > > Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: > 1) In true dom0less environment, dom0 would not exist > 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. Definitely we need to consider the case where there is no Dom0. I think there are in fact 3 different use cases here: - watchdog fully driven in a domain (dom0 or another): would work if it is expected that Xen + Domain boot time is under the watchdog initial refresh rate. I think this could make sense on some applications where your system depends on a specific domain to be properly booted to work. This would require an initial refresh time configurable in the boot loader probably. - watchdog fully driven by Xen. One might want to just make sure the hypervisor is alive. - hybrid model where the watchdog is driven by Xen until a domain comes up to drive it. This could make sense to relax the stress on boot time but would raise the question of what should be done if the domain dies. This is also kind of complex as Xen should stop refreshing the watchdog when a domain starts doing it (might require a trap and emulate initially that is then mapped directly to a domain). I am not completely sure this makes sense. Regards Bertrand > > Let see what the other Arm maintainer thinks. > > -- > Julien Grall
Hi Bertrand, On 3/13/24 11:07, Bertrand Marquis wrote: > Hi, > >> On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: >> >> Hi John, >> >> Thank you for the reply. >> >> On 08/03/2024 13:40, John Ernberg wrote: >>> On 3/7/24 00:07, Julien Grall wrote: >>>> > Ping on the watchdog discussion bits. >>>> >>>> Sorry for the late reply. >>>> >>>> On 06/03/2024 13:13, John Ernberg wrote: >>>>> On 2/9/24 14:14, John Ernberg wrote: >>>>>> >>>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>>> call will be used by Xen. >>>>>> >>>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>>> if it is being used. >>>>>> >>>>>> I looked around if there was previous discussion and only really >>>>>> found [3]. >>>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>>> x86 nor ACPI. >>>> >>>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>>> also have per-VM watchdog and this is configured by the hypercall >>>> SCHEDOP_watchdog. >>>> >>>>>> >>>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>>> >>>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>>> watchdog. So I think it would make more sense if this is not exposed to >>>> dom0 (even if Xen is doing nothing with it). >>>> >>>> Can you try to hide the SMCs and check if dom0 still behave properly? >>>> >>>> Cheers, >>>> >>> This SMC manages a hardware watchdog, if it's not pinged within a >>> specific interval the entire board resets. >> >> Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? >> >>> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >>> watchdog, triggering a board reset because the system looks to have >>> become unresponsive. The reason this patch set started is because we >>> couldn't ping the watchdog when running with Xen. >>> In our specific system the bootloader enables the watchdog as early as >>> possible so that we can get watchdog protection for as much of the boot >>> as we possibly can. >>> So, if we are to block the SMC from Dom0, then Xen needs to take over >>> the pinging. It could be implemented similarly to the NMI watchdog, >>> except that the system will reset if the ping is missed rather than >>> backtrace. >>> It would also mean that Xen will get a whole watchdog driver-category >>> due to the watchdog being vendor and sometimes even SoC specific when it >>> comes to Arm. >>> My understanding of the domain watchdog code is that today the domain >>> needs to call SCHEDOP_watchdog at least once to start the watchdog >>> timer. Since watchdog protection through the whole boot process is >>> desirable we'd need some core changes, such as an option to start the >>> domain watchdog on init. > >>> It's quite a big change to make >> >> For clarification, above you seem to mention two changes: >> >> 1) Allow Xen to use the HW watchdog >> 2) Allow the domain to use the watchdog early >> >> I am assuming by big change, you are referring to 2? >> >> , while I am not against doing it if it >>> makes sense, I now wonder if Xen should manage hardware watchdogs. >>> Looking at the domain watchdog code it looks like if a domain does not >>> get enough execution time, the watchdog will not be pinged enough and >>> the guest will be reset. So either watchdog approach requires Dom0 to >>> get execution time. Dom0 also needs to service all the PV backends it's >>> responsible for. I'm not sure it's valuable to add another layer of >>> watchdog for this scenario as the end result (checking that the entire >>> system works) is achieved without it as well. >>> So, before I try to find the time to make a proposal for moving the >>> hardware watchdog bit to Xen, do we really want it? >> >> Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: >> 1) In true dom0less environment, dom0 would not exist >> 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. > > Definitely we need to consider the case where there is no Dom0. > > I think there are in fact 3 different use cases here: > - watchdog fully driven in a domain (dom0 or another): would work if it is expected > that Xen + Domain boot time is under the watchdog initial refresh rate. I think this > could make sense on some applications where your system depends on a specific > domain to be properly booted to work. This would require an initial refresh time > configurable in the boot loader probably. This is our use-case. ^ Our dom0 is monitoring and managing the other domains in our system. Without dom0 working the system isn't really working as a whole. @Julien: Would you be ok with the patch set continuing in the direction of the original proposal, letting another party (or me at a later time) implement the fully driven by Xen option? > - watchdog fully driven by Xen. One might want to just make sure the hypervisor is alive. > - hybrid model where the watchdog is driven by Xen until a domain comes up to drive it. > This could make sense to relax the stress on boot time but would raise the question of > what should be done if the domain dies. This is also kind of complex as Xen should stop > refreshing the watchdog when a domain starts doing it (might require a trap and emulate > initially that is then mapped directly to a domain). I am not completely sure this makes sense. > > Regards > Bertrand > Thanks and best regards // John Ernberg
Hi John, On 20/03/2024 16:24, John Ernberg wrote: > Hi Bertrand, > > On 3/13/24 11:07, Bertrand Marquis wrote: >> Hi, >> >>> On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: >>> >>> Hi John, >>> >>> Thank you for the reply. >>> >>> On 08/03/2024 13:40, John Ernberg wrote: >>>> On 3/7/24 00:07, Julien Grall wrote: >>>>> > Ping on the watchdog discussion bits. >>>>> >>>>> Sorry for the late reply. >>>>> >>>>> On 06/03/2024 13:13, John Ernberg wrote: >>>>>> On 2/9/24 14:14, John Ernberg wrote: >>>>>>> >>>>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>>>> call will be used by Xen. >>>>>>> >>>>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>>>> if it is being used. >>>>>>> >>>>>>> I looked around if there was previous discussion and only really >>>>>>> found [3]. >>>>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>>>> x86 nor ACPI. >>>>> >>>>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>>>> also have per-VM watchdog and this is configured by the hypercall >>>>> SCHEDOP_watchdog. >>>>> >>>>>>> >>>>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>>>> >>>>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>>>> watchdog. So I think it would make more sense if this is not exposed to >>>>> dom0 (even if Xen is doing nothing with it). >>>>> >>>>> Can you try to hide the SMCs and check if dom0 still behave properly? >>>>> >>>>> Cheers, >>>>> >>>> This SMC manages a hardware watchdog, if it's not pinged within a >>>> specific interval the entire board resets. >>> >>> Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? >>> >>>> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >>>> watchdog, triggering a board reset because the system looks to have >>>> become unresponsive. The reason this patch set started is because we >>>> couldn't ping the watchdog when running with Xen. >>>> In our specific system the bootloader enables the watchdog as early as >>>> possible so that we can get watchdog protection for as much of the boot >>>> as we possibly can. >>>> So, if we are to block the SMC from Dom0, then Xen needs to take over >>>> the pinging. It could be implemented similarly to the NMI watchdog, >>>> except that the system will reset if the ping is missed rather than >>>> backtrace. >>>> It would also mean that Xen will get a whole watchdog driver-category >>>> due to the watchdog being vendor and sometimes even SoC specific when it >>>> comes to Arm. >>>> My understanding of the domain watchdog code is that today the domain >>>> needs to call SCHEDOP_watchdog at least once to start the watchdog >>>> timer. Since watchdog protection through the whole boot process is >>>> desirable we'd need some core changes, such as an option to start the >>>> domain watchdog on init. > >>>> It's quite a big change to make >>> >>> For clarification, above you seem to mention two changes: >>> >>> 1) Allow Xen to use the HW watchdog >>> 2) Allow the domain to use the watchdog early >>> >>> I am assuming by big change, you are referring to 2? >>> >>> , while I am not against doing it if it >>>> makes sense, I now wonder if Xen should manage hardware watchdogs. >>>> Looking at the domain watchdog code it looks like if a domain does not >>>> get enough execution time, the watchdog will not be pinged enough and >>>> the guest will be reset. So either watchdog approach requires Dom0 to >>>> get execution time. Dom0 also needs to service all the PV backends it's >>>> responsible for. I'm not sure it's valuable to add another layer of >>>> watchdog for this scenario as the end result (checking that the entire >>>> system works) is achieved without it as well. >>>> So, before I try to find the time to make a proposal for moving the >>>> hardware watchdog bit to Xen, do we really want it? >>> >>> Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: >>> 1) In true dom0less environment, dom0 would not exist >>> 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. >> >> Definitely we need to consider the case where there is no Dom0. >> >> I think there are in fact 3 different use cases here: >> - watchdog fully driven in a domain (dom0 or another): would work if it is expected >> that Xen + Domain boot time is under the watchdog initial refresh rate. I think this >> could make sense on some applications where your system depends on a specific >> domain to be properly booted to work. This would require an initial refresh time >> configurable in the boot loader probably. > > This is our use-case. ^ > > Our dom0 is monitoring and managing the other domains in our system. > Without dom0 working the system isn't really working as a whole. > > @Julien: Would you be ok with the patch set continuing in the direction > of the > original proposal, letting another party (or me at a later time) implement > the fully driven by Xen option? I am concerned about this particular point from Bertrand: "would work if it is expected that Xen + Domain boot time is under the watchdog initial refresh rate." How will a user be able to figure out how to initially configure the watchdog? Is this something you can easily configure in the bootloader at runtime? Overall, I am not for this approach at least in the current status. I would be more inclined if there are some documentations explaining how this is supposed to be configured on NXP, so others can use the code. Anyway, this is why we have multiple Arm maintainers for this kind of situation. If they other agrees with you, then they can ack the patch and this can be merged. Cheers,
On Wed, 20 Mar 2024, Julien Grall wrote: > Hi John, > > On 20/03/2024 16:24, John Ernberg wrote: > > Hi Bertrand, > > > > On 3/13/24 11:07, Bertrand Marquis wrote: > > > Hi, > > > > > > > On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: > > > > > > > > Hi John, > > > > > > > > Thank you for the reply. > > > > > > > > On 08/03/2024 13:40, John Ernberg wrote: > > > > > On 3/7/24 00:07, Julien Grall wrote: > > > > > > > Ping on the watchdog discussion bits. > > > > > > > > > > > > Sorry for the late reply. > > > > > > > > > > > > On 06/03/2024 13:13, John Ernberg wrote: > > > > > > > On 2/9/24 14:14, John Ernberg wrote: > > > > > > > > > > > > > > > > > * IMX_SIP_TIMER_*: This seems to be related to the > > > > > > > > > watchdog. > > > > > > > > > Shouldn't dom0 rely on the watchdog provided by Xen instead? > > > > > > > > > So those > > > > > > > > > call will be used by Xen. > > > > > > > > > > > > > > > > That is indeed a watchdog SIP, and also for setting the SoC > > > > > > > > internal RTC > > > > > > > > if it is being used. > > > > > > > > > > > > > > > > I looked around if there was previous discussion and only really > > > > > > > > found [3]. > > > > > > > > Is the xen/xen/include/watchdog.h header meant to be for this > > > > > > > > kind of > > > > > > > > watchdog support or is that more for the VM watchdog? Looking at > > > > > > > > the x86 > > > > > > > > ACPI NMI watchdog it seems like the former, but I have never > > > > > > > > worked with > > > > > > > > x86 nor ACPI. > > > > > > > > > > > > include/watchdog.h contains helper to configure the watchdog for > > > > > > Xen. We > > > > > > also have per-VM watchdog and this is configured by the hypercall > > > > > > SCHEDOP_watchdog. > > > > > > > > > > > > > > > > > > > > > > Currently forwarding it to Dom0 has not caused any watchdog > > > > > > > > resets with > > > > > > > > our watchdog timeout settings, our specific Dom0 setup and VM > > > > > > > > count. > > > > > > > > > > > > IIUC, the SMC API for the watchdog would be similar to the ACPI NMI > > > > > > watchdog. So I think it would make more sense if this is not exposed > > > > > > to > > > > > > dom0 (even if Xen is doing nothing with it). > > > > > > > > > > > > Can you try to hide the SMCs and check if dom0 still behave > > > > > > properly? > > > > > > > > > > > > Cheers, > > > > > > > > > > > This SMC manages a hardware watchdog, if it's not pinged within a > > > > > specific interval the entire board resets. > > > > > > > > Do you know what's the default interval? Is it large enough so Xen + > > > > dom0 can boot (at least up to when the watchdog driver is initialized)? > > > > > > > > > If I block the SMCs the watchdog driver in Dom0 will fail to ping the > > > > > watchdog, triggering a board reset because the system looks to have > > > > > become unresponsive. The reason this patch set started is because we > > > > > couldn't ping the watchdog when running with Xen. > > > > > In our specific system the bootloader enables the watchdog as early as > > > > > possible so that we can get watchdog protection for as much of the > > > > > boot > > > > > as we possibly can. > > > > > So, if we are to block the SMC from Dom0, then Xen needs to take over > > > > > the pinging. It could be implemented similarly to the NMI watchdog, > > > > > except that the system will reset if the ping is missed rather than > > > > > backtrace. > > > > > It would also mean that Xen will get a whole watchdog driver-category > > > > > due to the watchdog being vendor and sometimes even SoC specific when > > > > > it > > > > > comes to Arm. > > > > > My understanding of the domain watchdog code is that today the domain > > > > > needs to call SCHEDOP_watchdog at least once to start the watchdog > > > > > timer. Since watchdog protection through the whole boot process is > > > > > desirable we'd need some core changes, such as an option to start the > > > > > domain watchdog on init. > > > > > > It's quite a big change to make > > > > > > > > For clarification, above you seem to mention two changes: > > > > > > > > 1) Allow Xen to use the HW watchdog > > > > 2) Allow the domain to use the watchdog early > > > > > > > > I am assuming by big change, you are referring to 2? > > > > > > > > , while I am not against doing it if it > > > > > makes sense, I now wonder if Xen should manage hardware watchdogs. > > > > > Looking at the domain watchdog code it looks like if a domain does not > > > > > get enough execution time, the watchdog will not be pinged enough and > > > > > the guest will be reset. So either watchdog approach requires Dom0 to > > > > > get execution time. Dom0 also needs to service all the PV backends > > > > > it's > > > > > responsible for. I'm not sure it's valuable to add another layer of > > > > > watchdog for this scenario as the end result (checking that the entire > > > > > system works) is achieved without it as well. > > > > > So, before I try to find the time to make a proposal for moving the > > > > > hardware watchdog bit to Xen, do we really want it? > > > > > > > > Thanks for the details. Given that the watchdog is enabled by the > > > > bootloader, I think we want Xen to drive the watchdog for two reasons: > > > > 1) In true dom0less environment, dom0 would not exist > > > > 2) You are relying on Xen + Dom0 to boot (or at least enough to get the > > > > watchdog working) within the watchdog interval. > > > > > > Definitely we need to consider the case where there is no Dom0. > > > > > > I think there are in fact 3 different use cases here: > > > - watchdog fully driven in a domain (dom0 or another): would work if it is > > > expected > > > that Xen + Domain boot time is under the watchdog initial refresh > > > rate. I think this > > > could make sense on some applications where your system depends on a > > > specific > > > domain to be properly booted to work. This would require an initial > > > refresh time > > > configurable in the boot loader probably. > > > > This is our use-case. ^ > > > > Our dom0 is monitoring and managing the other domains in our system. > > Without dom0 working the system isn't really working as a whole. > > > > @Julien: Would you be ok with the patch set continuing in the direction > > of the > > original proposal, letting another party (or me at a later time) implement > > the fully driven by Xen option? > I am concerned about this particular point from Bertrand: > > "would work if it is expected that Xen + Domain boot time is under the > watchdog initial refresh rate." > > How will a user be able to figure out how to initially configure the watchdog? > Is this something you can easily configure in the bootloader at runtime? > > > Overall, I am not for this approach at least in the current status. I would be > more inclined if there are some documentations explaining how this is supposed > to be configured on NXP, so others can use the code. > > Anyway, this is why we have multiple Arm maintainers for this kind of > situation. If they other agrees with you, then they can ack the patch and this > can be merged. The approach here would not be my choice either. However, I think it would be nice to have better support for NXP imx8 boards in upstream Xen. To that end, I would ack these patches but I would ask to add a document under xen.git/docs/ explaining the approach, limitations, and requirements, so that someone else can use the code effectively.
> Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue > > On Wed, 20 Mar 2024, Julien Grall wrote: > > Hi John, > > > > On 20/03/2024 16:24, John Ernberg wrote: > > > Hi Bertrand, > > > > > > On 3/13/24 11:07, Bertrand Marquis wrote: > > > > Hi, > > > > > > > > > On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: > > > > > > > > > > Hi John, > > > > > > > > > > Thank you for the reply. > > > > > > > > > > On 08/03/2024 13:40, John Ernberg wrote: > > > > > > On 3/7/24 00:07, Julien Grall wrote: > > > > > > > > Ping on the watchdog discussion bits. > > > > > > > > > > > > > > Sorry for the late reply. > > > > > > > > > > > > > > On 06/03/2024 13:13, John Ernberg wrote: > > > > > > > > On 2/9/24 14:14, John Ernberg wrote: > > > > > > > > > > > > > > > > > > > * IMX_SIP_TIMER_*: This seems to be related to > > > > > > > > > > the watchdog. > > > > > > > > > > Shouldn't dom0 rely on the watchdog provided by Xen > instead? > > > > > > > > > > So those > > > > > > > > > > call will be used by Xen. > > > > > > > > > > > > > > > > > > That is indeed a watchdog SIP, and also for setting the > > > > > > > > > SoC internal RTC if it is being used. > > > > > > > > > > > > > > > > > > I looked around if there was previous discussion and > > > > > > > > > only really found [3]. > > > > > > > > > Is the xen/xen/include/watchdog.h header meant to be for > > > > > > > > > this kind of watchdog support or is that more for the VM > > > > > > > > > watchdog? Looking at the x86 ACPI NMI watchdog it seems > > > > > > > > > like the former, but I have never worked with > > > > > > > > > x86 nor ACPI. > > > > > > > > > > > > > > include/watchdog.h contains helper to configure the watchdog > > > > > > > for Xen. We also have per-VM watchdog and this is configured > > > > > > > by the hypercall SCHEDOP_watchdog. > > > > > > > > > > > > > > > > > > > > > > > > > Currently forwarding it to Dom0 has not caused any > > > > > > > > > watchdog resets with our watchdog timeout settings, our > > > > > > > > > specific Dom0 setup and VM count. > > > > > > > > > > > > > > IIUC, the SMC API for the watchdog would be similar to the > > > > > > > ACPI NMI watchdog. So I think it would make more sense if > > > > > > > this is not exposed to > > > > > > > dom0 (even if Xen is doing nothing with it). > > > > > > > > > > > > > > Can you try to hide the SMCs and check if dom0 still behave > > > > > > > properly? > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > This SMC manages a hardware watchdog, if it's not pinged > > > > > > within a specific interval the entire board resets. > > > > > > > > > > Do you know what's the default interval? Is it large enough so > > > > > Xen + > > > > > dom0 can boot (at least up to when the watchdog driver is initialized)? > > > > > > > > > > > If I block the SMCs the watchdog driver in Dom0 will fail to > > > > > > ping the watchdog, triggering a board reset because the system > > > > > > looks to have become unresponsive. The reason this patch set > > > > > > started is because we couldn't ping the watchdog when running with > Xen. > > > > > > In our specific system the bootloader enables the watchdog as > > > > > > early as possible so that we can get watchdog protection for > > > > > > as much of the boot as we possibly can. > > > > > > So, if we are to block the SMC from Dom0, then Xen needs to > > > > > > take over the pinging. It could be implemented similarly to > > > > > > the NMI watchdog, except that the system will reset if the > > > > > > ping is missed rather than backtrace. > > > > > > It would also mean that Xen will get a whole watchdog > > > > > > driver-category due to the watchdog being vendor and sometimes > > > > > > even SoC specific when it comes to Arm. > > > > > > My understanding of the domain watchdog code is that today the > > > > > > domain needs to call SCHEDOP_watchdog at least once to start > > > > > > the watchdog timer. Since watchdog protection through the > > > > > > whole boot process is desirable we'd need some core changes, > > > > > > such as an option to start the domain watchdog on init. > It's > > > > > > quite a big change to make > > > > > > > > > > For clarification, above you seem to mention two changes: > > > > > > > > > > 1) Allow Xen to use the HW watchdog > > > > > 2) Allow the domain to use the watchdog early > > > > > > > > > > I am assuming by big change, you are referring to 2? > > > > > > > > > > , while I am not against doing it if it > > > > > > makes sense, I now wonder if Xen should manage hardware > watchdogs. > > > > > > Looking at the domain watchdog code it looks like if a domain > > > > > > does not get enough execution time, the watchdog will not be > > > > > > pinged enough and the guest will be reset. So either watchdog > > > > > > approach requires Dom0 to get execution time. Dom0 also needs > > > > > > to service all the PV backends it's responsible for. I'm not > > > > > > sure it's valuable to add another layer of watchdog for this > > > > > > scenario as the end result (checking that the entire system > > > > > > works) is achieved without it as well. > > > > > > So, before I try to find the time to make a proposal for > > > > > > moving the hardware watchdog bit to Xen, do we really want it? > > > > > > > > > > Thanks for the details. Given that the watchdog is enabled by > > > > > the bootloader, I think we want Xen to drive the watchdog for two > reasons: > > > > > 1) In true dom0less environment, dom0 would not exist > > > > > 2) You are relying on Xen + Dom0 to boot (or at least enough to > > > > > get the watchdog working) within the watchdog interval. > > > > > > > > Definitely we need to consider the case where there is no Dom0. > > > > > > > > I think there are in fact 3 different use cases here: > > > > - watchdog fully driven in a domain (dom0 or another): would work > > > > if it is expected > > > > that Xen + Domain boot time is under the watchdog initial > > > > refresh rate. I think this > > > > could make sense on some applications where your system > > > > depends on a specific > > > > domain to be properly booted to work. This would require an > > > > initial refresh time > > > > configurable in the boot loader probably. > > > > > > This is our use-case. ^ > > > > > > Our dom0 is monitoring and managing the other domains in our system. > > > Without dom0 working the system isn't really working as a whole. > > > > > > @Julien: Would you be ok with the patch set continuing in the > > > direction of the original proposal, letting another party (or me at > > > a later time) implement the fully driven by Xen option? > > I am concerned about this particular point from Bertrand: > > > > "would work if it is expected that Xen + Domain boot time is under the > > watchdog initial refresh rate." > > > > How will a user be able to figure out how to initially configure the watchdog? > > Is this something you can easily configure in the bootloader at runtime? > > > > > > Overall, I am not for this approach at least in the current status. I > > would be more inclined if there are some documentations explaining how > > this is supposed to be configured on NXP, so others can use the code. I will try to push inside NXP to release a documentation on SIP usage. But not expect it will be released soon. The SIP number is quite stable, and we not change the meaning. Regards, Peng. > > > > Anyway, this is why we have multiple Arm maintainers for this kind of > > situation. If they other agrees with you, then they can ack the patch > > and this can be merged. > > > The approach here would not be my choice either. However, I think it would > be nice to have better support for NXP imx8 boards in upstream Xen. To that > end, I would ack these patches but I would ask to add a document under > xen.git/docs/ explaining the approach, limitations, and requirements, so that > someone else can use the code effectively.
Hi, > On 20 Mar 2024, at 18:40, Julien Grall <julien@xen.org> wrote: > > Hi John, > > On 20/03/2024 16:24, John Ernberg wrote: >> Hi Bertrand, >> On 3/13/24 11:07, Bertrand Marquis wrote: >>> Hi, >>> >>>> On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: >>>> >>>> Hi John, >>>> >>>> Thank you for the reply. >>>> >>>> On 08/03/2024 13:40, John Ernberg wrote: >>>>> On 3/7/24 00:07, Julien Grall wrote: >>>>>> > Ping on the watchdog discussion bits. >>>>>> >>>>>> Sorry for the late reply. >>>>>> >>>>>> On 06/03/2024 13:13, John Ernberg wrote: >>>>>>> On 2/9/24 14:14, John Ernberg wrote: >>>>>>>> >>>>>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>>>>> call will be used by Xen. >>>>>>>> >>>>>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>>>>> if it is being used. >>>>>>>> >>>>>>>> I looked around if there was previous discussion and only really >>>>>>>> found [3]. >>>>>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>>>>> x86 nor ACPI. >>>>>> >>>>>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>>>>> also have per-VM watchdog and this is configured by the hypercall >>>>>> SCHEDOP_watchdog. >>>>>> >>>>>>>> >>>>>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>>>>> >>>>>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>>>>> watchdog. So I think it would make more sense if this is not exposed to >>>>>> dom0 (even if Xen is doing nothing with it). >>>>>> >>>>>> Can you try to hide the SMCs and check if dom0 still behave properly? >>>>>> >>>>>> Cheers, >>>>>> >>>>> This SMC manages a hardware watchdog, if it's not pinged within a >>>>> specific interval the entire board resets. >>>> >>>> Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? >>>> >>>>> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >>>>> watchdog, triggering a board reset because the system looks to have >>>>> become unresponsive. The reason this patch set started is because we >>>>> couldn't ping the watchdog when running with Xen. >>>>> In our specific system the bootloader enables the watchdog as early as >>>>> possible so that we can get watchdog protection for as much of the boot >>>>> as we possibly can. >>>>> So, if we are to block the SMC from Dom0, then Xen needs to take over >>>>> the pinging. It could be implemented similarly to the NMI watchdog, >>>>> except that the system will reset if the ping is missed rather than >>>>> backtrace. >>>>> It would also mean that Xen will get a whole watchdog driver-category >>>>> due to the watchdog being vendor and sometimes even SoC specific when it >>>>> comes to Arm. >>>>> My understanding of the domain watchdog code is that today the domain >>>>> needs to call SCHEDOP_watchdog at least once to start the watchdog >>>>> timer. Since watchdog protection through the whole boot process is >>>>> desirable we'd need some core changes, such as an option to start the >>>>> domain watchdog on init. > >>>>> It's quite a big change to make >>>> >>>> For clarification, above you seem to mention two changes: >>>> >>>> 1) Allow Xen to use the HW watchdog >>>> 2) Allow the domain to use the watchdog early >>>> >>>> I am assuming by big change, you are referring to 2? >>>> >>>> , while I am not against doing it if it >>>>> makes sense, I now wonder if Xen should manage hardware watchdogs. >>>>> Looking at the domain watchdog code it looks like if a domain does not >>>>> get enough execution time, the watchdog will not be pinged enough and >>>>> the guest will be reset. So either watchdog approach requires Dom0 to >>>>> get execution time. Dom0 also needs to service all the PV backends it's >>>>> responsible for. I'm not sure it's valuable to add another layer of >>>>> watchdog for this scenario as the end result (checking that the entire >>>>> system works) is achieved without it as well. >>>>> So, before I try to find the time to make a proposal for moving the >>>>> hardware watchdog bit to Xen, do we really want it? >>>> >>>> Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: >>>> 1) In true dom0less environment, dom0 would not exist >>>> 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. >>> >>> Definitely we need to consider the case where there is no Dom0. >>> >>> I think there are in fact 3 different use cases here: >>> - watchdog fully driven in a domain (dom0 or another): would work if it is expected >>> that Xen + Domain boot time is under the watchdog initial refresh rate. I think this >>> could make sense on some applications where your system depends on a specific >>> domain to be properly booted to work. This would require an initial refresh time >>> configurable in the boot loader probably. >> This is our use-case. ^ >> Our dom0 is monitoring and managing the other domains in our system. >> Without dom0 working the system isn't really working as a whole. >> @Julien: Would you be ok with the patch set continuing in the direction >> of the >> original proposal, letting another party (or me at a later time) implement >> the fully driven by Xen option? > I am concerned about this particular point from Bertrand: > > "would work if it is expected that Xen + Domain boot time is under the watchdog initial refresh rate." > > How will a user be able to figure out how to initially configure the watchdog? Is this something you can easily configure in the bootloader at runtime? Definitely here it would be better to have the watchdog turned off by default and document how to enable it in the firmware. Even if a long timeout is configured by default, a user could run into trouble if using a linux without watchdog or not running linux or using dom0less without a linux having access to it. I agree with Julien here that the concern could be that users would come to us instead of NXP if there is system is doing a reset without reasons after some seconds or minutes. > > > Overall, I am not for this approach at least in the current status. I would be more inclined if there are some documentations explaining how this is supposed to be configured on NXP, so others can use the code. > > Anyway, this is why we have multiple Arm maintainers for this kind of situation. If they other agrees with you, then they can ack the patch and this can be merged. I agree with Stefano that it would be good to have those board supported. One thing i could suggest until there is a watchdog driver inside Xen is to have a clear Warning at Xen boot on those boards in the console so that we could at least identify the reason easily if a reset occurs for someone. Cheers Bertrand > > Cheers, > > -- > Julien Grall
Hi Bertrand, On 3/21/24 08:41, Bertrand Marquis wrote: > Hi, > >> On 20 Mar 2024, at 18:40, Julien Grall <julien@xen.org> wrote: >> >> Hi John, >> >> On 20/03/2024 16:24, John Ernberg wrote: >>> Hi Bertrand, >>> On 3/13/24 11:07, Bertrand Marquis wrote: >>>> Hi, >>>> >>>>> On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: >>>>> >>>>> Hi John, >>>>> >>>>> Thank you for the reply. >>>>> >>>>> On 08/03/2024 13:40, John Ernberg wrote: >>>>>> On 3/7/24 00:07, Julien Grall wrote: >>>>>>> > Ping on the watchdog discussion bits. >>>>>>> >>>>>>> Sorry for the late reply. >>>>>>> >>>>>>> On 06/03/2024 13:13, John Ernberg wrote: >>>>>>>> On 2/9/24 14:14, John Ernberg wrote: >>>>>>>>> >>>>>>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>>>>>> call will be used by Xen. >>>>>>>>> >>>>>>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>>>>>> if it is being used. >>>>>>>>> >>>>>>>>> I looked around if there was previous discussion and only really >>>>>>>>> found [3]. >>>>>>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>>>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>>>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>>>>>> x86 nor ACPI. >>>>>>> >>>>>>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>>>>>> also have per-VM watchdog and this is configured by the hypercall >>>>>>> SCHEDOP_watchdog. >>>>>>> >>>>>>>>> >>>>>>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>>>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>>>>>> >>>>>>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>>>>>> watchdog. So I think it would make more sense if this is not exposed to >>>>>>> dom0 (even if Xen is doing nothing with it). >>>>>>> >>>>>>> Can you try to hide the SMCs and check if dom0 still behave properly? >>>>>>> >>>>>>> Cheers, >>>>>>> >>>>>> This SMC manages a hardware watchdog, if it's not pinged within a >>>>>> specific interval the entire board resets. >>>>> >>>>> Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? >>>>> >>>>>> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >>>>>> watchdog, triggering a board reset because the system looks to have >>>>>> become unresponsive. The reason this patch set started is because we >>>>>> couldn't ping the watchdog when running with Xen. >>>>>> In our specific system the bootloader enables the watchdog as early as >>>>>> possible so that we can get watchdog protection for as much of the boot >>>>>> as we possibly can. >>>>>> So, if we are to block the SMC from Dom0, then Xen needs to take over >>>>>> the pinging. It could be implemented similarly to the NMI watchdog, >>>>>> except that the system will reset if the ping is missed rather than >>>>>> backtrace. >>>>>> It would also mean that Xen will get a whole watchdog driver-category >>>>>> due to the watchdog being vendor and sometimes even SoC specific when it >>>>>> comes to Arm. >>>>>> My understanding of the domain watchdog code is that today the domain >>>>>> needs to call SCHEDOP_watchdog at least once to start the watchdog >>>>>> timer. Since watchdog protection through the whole boot process is >>>>>> desirable we'd need some core changes, such as an option to start the >>>>>> domain watchdog on init. > >>>>>> It's quite a big change to make >>>>> >>>>> For clarification, above you seem to mention two changes: >>>>> >>>>> 1) Allow Xen to use the HW watchdog >>>>> 2) Allow the domain to use the watchdog early >>>>> >>>>> I am assuming by big change, you are referring to 2? >>>>> >>>>> , while I am not against doing it if it >>>>>> makes sense, I now wonder if Xen should manage hardware watchdogs. >>>>>> Looking at the domain watchdog code it looks like if a domain does not >>>>>> get enough execution time, the watchdog will not be pinged enough and >>>>>> the guest will be reset. So either watchdog approach requires Dom0 to >>>>>> get execution time. Dom0 also needs to service all the PV backends it's >>>>>> responsible for. I'm not sure it's valuable to add another layer of >>>>>> watchdog for this scenario as the end result (checking that the entire >>>>>> system works) is achieved without it as well. >>>>>> So, before I try to find the time to make a proposal for moving the >>>>>> hardware watchdog bit to Xen, do we really want it? >>>>> >>>>> Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: >>>>> 1) In true dom0less environment, dom0 would not exist >>>>> 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. >>>> >>>> Definitely we need to consider the case where there is no Dom0. >>>> >>>> I think there are in fact 3 different use cases here: >>>> - watchdog fully driven in a domain (dom0 or another): would work if it is expected >>>> that Xen + Domain boot time is under the watchdog initial refresh rate. I think this >>>> could make sense on some applications where your system depends on a specific >>>> domain to be properly booted to work. This would require an initial refresh time >>>> configurable in the boot loader probably. >>> This is our use-case. ^ >>> Our dom0 is monitoring and managing the other domains in our system. >>> Without dom0 working the system isn't really working as a whole. >>> @Julien: Would you be ok with the patch set continuing in the direction >>> of the >>> original proposal, letting another party (or me at a later time) implement >>> the fully driven by Xen option? >> I am concerned about this particular point from Bertrand: >> >> "would work if it is expected that Xen + Domain boot time is under the watchdog initial refresh rate." >> >> How will a user be able to figure out how to initially configure the watchdog? Is this something you can easily configure in the bootloader at runtime? If you go through the trouble of enabling the watchdog in the bootloader you usually know what you're doing and set the timeout yourself. Since our systems can be mounted in really annoying locations (both in installations and world-wise) we need as much auto-recovery as possible to avoid people having to travel to collect a unit that just needed a reset due to some unexpected obscure issue at startup. > > Definitely here it would be better to have the watchdog turned off by default and document how to enable it in the firmware. > > Even if a long timeout is configured by default, a user could run into trouble if using a linux without watchdog or not running linux or using dom0less without a linux having access to it. > I agree with Julien here that the concern could be that users would come to us instead of NXP if there is system is doing a reset without reasons after some seconds or minutes. I could add myself as a reviewer for the iMX parts if that helps routing such questions (and future patches) also to me. We have experience with the QXP, and the QM (for the supported parts by this patch set) is identical. Would that help with the concerns? > >> >> >> Overall, I am not for this approach at least in the current status. I would be more inclined if there are some documentations explaining how this is supposed to be configured on NXP, so others can use the code. >> >> Anyway, this is why we have multiple Arm maintainers for this kind of situation. If they other agrees with you, then they can ack the patch and this can be merged. > > I agree with Stefano that it would be good to have those board supported. > > One thing i could suggest until there is a watchdog driver inside Xen is to have a clear Warning at Xen boot on those boards in the console so that we could at least identify the reason easily if a reset occurs for someone. How do other SoCs deal with this currently? The iMX SoCs aren't the only ones with a watchdog, just the first one added to Xen that pings the watchdog over an SMC. What about the OMAPs, the R-Cars, Xilinx's, Exynos' and so on? Thanks! // John Ernberg
Hi John, > On 21 Mar 2024, at 17:05, John Ernberg <john.ernberg@actia.se> wrote: > > Hi Bertrand, > > On 3/21/24 08:41, Bertrand Marquis wrote: >> Hi, >> >>> On 20 Mar 2024, at 18:40, Julien Grall <julien@xen.org> wrote: >>> >>> Hi John, >>> >>> On 20/03/2024 16:24, John Ernberg wrote: >>>> Hi Bertrand, >>>> On 3/13/24 11:07, Bertrand Marquis wrote: >>>>> Hi, >>>>> >>>>>> On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: >>>>>> >>>>>> Hi John, >>>>>> >>>>>> Thank you for the reply. >>>>>> >>>>>> On 08/03/2024 13:40, John Ernberg wrote: >>>>>>> On 3/7/24 00:07, Julien Grall wrote: >>>>>>>>> Ping on the watchdog discussion bits. >>>>>>>> >>>>>>>> Sorry for the late reply. >>>>>>>> >>>>>>>> On 06/03/2024 13:13, John Ernberg wrote: >>>>>>>>> On 2/9/24 14:14, John Ernberg wrote: >>>>>>>>>> >>>>>>>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>>>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>>>>>>> call will be used by Xen. >>>>>>>>>> >>>>>>>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>>>>>>> if it is being used. >>>>>>>>>> >>>>>>>>>> I looked around if there was previous discussion and only really >>>>>>>>>> found [3]. >>>>>>>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>>>>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>>>>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>>>>>>> x86 nor ACPI. >>>>>>>> >>>>>>>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>>>>>>> also have per-VM watchdog and this is configured by the hypercall >>>>>>>> SCHEDOP_watchdog. >>>>>>>> >>>>>>>>>> >>>>>>>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>>>>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>>>>>>> >>>>>>>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>>>>>>> watchdog. So I think it would make more sense if this is not exposed to >>>>>>>> dom0 (even if Xen is doing nothing with it). >>>>>>>> >>>>>>>> Can you try to hide the SMCs and check if dom0 still behave properly? >>>>>>>> >>>>>>>> Cheers, >>>>>>>> >>>>>>> This SMC manages a hardware watchdog, if it's not pinged within a >>>>>>> specific interval the entire board resets. >>>>>> >>>>>> Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? >>>>>> >>>>>>> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >>>>>>> watchdog, triggering a board reset because the system looks to have >>>>>>> become unresponsive. The reason this patch set started is because we >>>>>>> couldn't ping the watchdog when running with Xen. >>>>>>> In our specific system the bootloader enables the watchdog as early as >>>>>>> possible so that we can get watchdog protection for as much of the boot >>>>>>> as we possibly can. >>>>>>> So, if we are to block the SMC from Dom0, then Xen needs to take over >>>>>>> the pinging. It could be implemented similarly to the NMI watchdog, >>>>>>> except that the system will reset if the ping is missed rather than >>>>>>> backtrace. >>>>>>> It would also mean that Xen will get a whole watchdog driver-category >>>>>>> due to the watchdog being vendor and sometimes even SoC specific when it >>>>>>> comes to Arm. >>>>>>> My understanding of the domain watchdog code is that today the domain >>>>>>> needs to call SCHEDOP_watchdog at least once to start the watchdog >>>>>>> timer. Since watchdog protection through the whole boot process is >>>>>>> desirable we'd need some core changes, such as an option to start the >>>>>>> domain watchdog on init. > >>>>>>> It's quite a big change to make >>>>>> >>>>>> For clarification, above you seem to mention two changes: >>>>>> >>>>>> 1) Allow Xen to use the HW watchdog >>>>>> 2) Allow the domain to use the watchdog early >>>>>> >>>>>> I am assuming by big change, you are referring to 2? >>>>>> >>>>>> , while I am not against doing it if it >>>>>>> makes sense, I now wonder if Xen should manage hardware watchdogs. >>>>>>> Looking at the domain watchdog code it looks like if a domain does not >>>>>>> get enough execution time, the watchdog will not be pinged enough and >>>>>>> the guest will be reset. So either watchdog approach requires Dom0 to >>>>>>> get execution time. Dom0 also needs to service all the PV backends it's >>>>>>> responsible for. I'm not sure it's valuable to add another layer of >>>>>>> watchdog for this scenario as the end result (checking that the entire >>>>>>> system works) is achieved without it as well. >>>>>>> So, before I try to find the time to make a proposal for moving the >>>>>>> hardware watchdog bit to Xen, do we really want it? >>>>>> >>>>>> Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: >>>>>> 1) In true dom0less environment, dom0 would not exist >>>>>> 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. >>>>> >>>>> Definitely we need to consider the case where there is no Dom0. >>>>> >>>>> I think there are in fact 3 different use cases here: >>>>> - watchdog fully driven in a domain (dom0 or another): would work if it is expected >>>>> that Xen + Domain boot time is under the watchdog initial refresh rate. I think this >>>>> could make sense on some applications where your system depends on a specific >>>>> domain to be properly booted to work. This would require an initial refresh time >>>>> configurable in the boot loader probably. >>>> This is our use-case. ^ >>>> Our dom0 is monitoring and managing the other domains in our system. >>>> Without dom0 working the system isn't really working as a whole. >>>> @Julien: Would you be ok with the patch set continuing in the direction >>>> of the >>>> original proposal, letting another party (or me at a later time) implement >>>> the fully driven by Xen option? >>> I am concerned about this particular point from Bertrand: >>> >>> "would work if it is expected that Xen + Domain boot time is under the watchdog initial refresh rate." >>> >>> How will a user be able to figure out how to initially configure the watchdog? Is this something you can easily configure in the bootloader at runtime? > > If you go through the trouble of enabling the watchdog in the bootloader you > usually know what you're doing and set the timeout yourself. > > Since our systems can be mounted in really annoying locations (both in > installations and world-wise) we need as much auto-recovery as possible to > avoid people having to travel to collect a unit that just needed a reset due > to some unexpected obscure issue at startup. I definitely get the need do not get me wrong. I am just concerned by potential users having target restarting when using Xen because of that and not knowing why. >> >> Definitely here it would be better to have the watchdog turned off by default and document how to enable it in the firmware. >> >> Even if a long timeout is configured by default, a user could run into trouble if using a linux without watchdog or not running linux or using dom0less without a linux having access to it. >> I agree with Julien here that the concern could be that users would come to us instead of NXP if there is system is doing a reset without reasons after some seconds or minutes. > > I could add myself as a reviewer for the iMX parts if that helps routing > such > questions (and future patches) also to me. We have experience with the QXP, > and the QM (for the supported parts by this patch set) is identical. > > Would that help with the concerns? Definitely it could help. > >> >>> >>> >>> Overall, I am not for this approach at least in the current status. I would be more inclined if there are some documentations explaining how this is supposed to be configured on NXP, so others can use the code. >>> >>> Anyway, this is why we have multiple Arm maintainers for this kind of situation. If they other agrees with you, then they can ack the patch and this can be merged. >> >> I agree with Stefano that it would be good to have those board supported. >> >> One thing i could suggest until there is a watchdog driver inside Xen is to have a clear Warning at Xen boot on those boards in the console so that we could at least identify the reason easily if a reset occurs for someone. > > How do other SoCs deal with this currently? The iMX SoCs aren't the only > ones > with a watchdog, just the first one added to Xen that pings the watchdog > over > an SMC. What about the OMAPs, the R-Cars, Xilinx's, Exynos' and so on? As far as I know the watchdog is usually not active until a driver activates it. Which means that by default it will not fire. Having it activated by the bootloader by default is not usual. Now definitely on a lot of use cases the users are activating it in the booloader but their systems are design for it. Do I understand that the default boot loader configuration is not activating it on your side ? Regards Bertrand > > Thanks! // John Ernberg
Hi Bertrand, On 3/21/24 17:15, Bertrand Marquis wrote: > Hi John, > >> On 21 Mar 2024, at 17:05, John Ernberg <john.ernberg@actia.se> wrote: >> >> Hi Bertrand, >> >> On 3/21/24 08:41, Bertrand Marquis wrote: >>> Hi, >>> >>>> On 20 Mar 2024, at 18:40, Julien Grall <julien@xen.org> wrote: >>>> >>>> Hi John, >>>> >>>> On 20/03/2024 16:24, John Ernberg wrote: >>>>> Hi Bertrand, >>>>> On 3/13/24 11:07, Bertrand Marquis wrote: >>>>>> Hi, >>>>>> >>>>>>> On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: >>>>>>> >>>>>>> Hi John, >>>>>>> >>>>>>> Thank you for the reply. >>>>>>> >>>>>>> On 08/03/2024 13:40, John Ernberg wrote: >>>>>>>> On 3/7/24 00:07, Julien Grall wrote: >>>>>>>>>> Ping on the watchdog discussion bits. >>>>>>>>> >>>>>>>>> Sorry for the late reply. >>>>>>>>> >>>>>>>>> On 06/03/2024 13:13, John Ernberg wrote: >>>>>>>>>> On 2/9/24 14:14, John Ernberg wrote: >>>>>>>>>>> >>>>>>>>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>>>>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>>>>>>>> call will be used by Xen. >>>>>>>>>>> >>>>>>>>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>>>>>>>> if it is being used. >>>>>>>>>>> >>>>>>>>>>> I looked around if there was previous discussion and only really >>>>>>>>>>> found [3]. >>>>>>>>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>>>>>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>>>>>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>>>>>>>> x86 nor ACPI. >>>>>>>>> >>>>>>>>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>>>>>>>> also have per-VM watchdog and this is configured by the hypercall >>>>>>>>> SCHEDOP_watchdog. >>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>>>>>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>>>>>>>> >>>>>>>>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>>>>>>>> watchdog. So I think it would make more sense if this is not exposed to >>>>>>>>> dom0 (even if Xen is doing nothing with it). >>>>>>>>> >>>>>>>>> Can you try to hide the SMCs and check if dom0 still behave properly? >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> >>>>>>>> This SMC manages a hardware watchdog, if it's not pinged within a >>>>>>>> specific interval the entire board resets. >>>>>>> >>>>>>> Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? >>>>>>> >>>>>>>> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >>>>>>>> watchdog, triggering a board reset because the system looks to have >>>>>>>> become unresponsive. The reason this patch set started is because we >>>>>>>> couldn't ping the watchdog when running with Xen. >>>>>>>> In our specific system the bootloader enables the watchdog as early as >>>>>>>> possible so that we can get watchdog protection for as much of the boot >>>>>>>> as we possibly can. >>>>>>>> So, if we are to block the SMC from Dom0, then Xen needs to take over >>>>>>>> the pinging. It could be implemented similarly to the NMI watchdog, >>>>>>>> except that the system will reset if the ping is missed rather than >>>>>>>> backtrace. >>>>>>>> It would also mean that Xen will get a whole watchdog driver-category >>>>>>>> due to the watchdog being vendor and sometimes even SoC specific when it >>>>>>>> comes to Arm. >>>>>>>> My understanding of the domain watchdog code is that today the domain >>>>>>>> needs to call SCHEDOP_watchdog at least once to start the watchdog >>>>>>>> timer. Since watchdog protection through the whole boot process is >>>>>>>> desirable we'd need some core changes, such as an option to start the >>>>>>>> domain watchdog on init. > >>>>>>>> It's quite a big change to make >>>>>>> >>>>>>> For clarification, above you seem to mention two changes: >>>>>>> >>>>>>> 1) Allow Xen to use the HW watchdog >>>>>>> 2) Allow the domain to use the watchdog early >>>>>>> >>>>>>> I am assuming by big change, you are referring to 2? >>>>>>> >>>>>>> , while I am not against doing it if it >>>>>>>> makes sense, I now wonder if Xen should manage hardware watchdogs. >>>>>>>> Looking at the domain watchdog code it looks like if a domain does not >>>>>>>> get enough execution time, the watchdog will not be pinged enough and >>>>>>>> the guest will be reset. So either watchdog approach requires Dom0 to >>>>>>>> get execution time. Dom0 also needs to service all the PV backends it's >>>>>>>> responsible for. I'm not sure it's valuable to add another layer of >>>>>>>> watchdog for this scenario as the end result (checking that the entire >>>>>>>> system works) is achieved without it as well. >>>>>>>> So, before I try to find the time to make a proposal for moving the >>>>>>>> hardware watchdog bit to Xen, do we really want it? >>>>>>> >>>>>>> Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: >>>>>>> 1) In true dom0less environment, dom0 would not exist >>>>>>> 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. >>>>>> >>>>>> Definitely we need to consider the case where there is no Dom0. >>>>>> >>>>>> I think there are in fact 3 different use cases here: >>>>>> - watchdog fully driven in a domain (dom0 or another): would work if it is expected >>>>>> that Xen + Domain boot time is under the watchdog initial refresh rate. I think this >>>>>> could make sense on some applications where your system depends on a specific >>>>>> domain to be properly booted to work. This would require an initial refresh time >>>>>> configurable in the boot loader probably. >>>>> This is our use-case. ^ >>>>> Our dom0 is monitoring and managing the other domains in our system. >>>>> Without dom0 working the system isn't really working as a whole. >>>>> @Julien: Would you be ok with the patch set continuing in the direction >>>>> of the >>>>> original proposal, letting another party (or me at a later time) implement >>>>> the fully driven by Xen option? >>>> I am concerned about this particular point from Bertrand: >>>> >>>> "would work if it is expected that Xen + Domain boot time is under the watchdog initial refresh rate." >>>> >>>> How will a user be able to figure out how to initially configure the watchdog? Is this something you can easily configure in the bootloader at runtime? >> >> If you go through the trouble of enabling the watchdog in the bootloader you >> usually know what you're doing and set the timeout yourself. >> >> Since our systems can be mounted in really annoying locations (both in >> installations and world-wise) we need as much auto-recovery as possible to >> avoid people having to travel to collect a unit that just needed a reset due >> to some unexpected obscure issue at startup. > > I definitely get the need do not get me wrong. > I am just concerned by potential users having target restarting when using Xen because of that and not knowing why. > >>> >>> Definitely here it would be better to have the watchdog turned off by default and document how to enable it in the firmware. >>> >>> Even if a long timeout is configured by default, a user could run into trouble if using a linux without watchdog or not running linux or using dom0less without a linux having access to it. >>> I agree with Julien here that the concern could be that users would come to us instead of NXP if there is system is doing a reset without reasons after some seconds or minutes. >> >> I could add myself as a reviewer for the iMX parts if that helps routing >> such >> questions (and future patches) also to me. We have experience with the QXP, >> and the QM (for the supported parts by this patch set) is identical. >> >> Would that help with the concerns? > > Definitely it could help. I'll figure out how to include myself in the MAINTAINERS file for the next spin. > >> >>> >>>> >>>> >>>> Overall, I am not for this approach at least in the current status. I would be more inclined if there are some documentations explaining how this is supposed to be configured on NXP, so others can use the code. >>>> >>>> Anyway, this is why we have multiple Arm maintainers for this kind of situation. If they other agrees with you, then they can ack the patch and this can be merged. >>> >>> I agree with Stefano that it would be good to have those board supported. >>> >>> One thing i could suggest until there is a watchdog driver inside Xen is to have a clear Warning at Xen boot on those boards in the console so that we could at least identify the reason easily if a reset occurs for someone. >> >> How do other SoCs deal with this currently? The iMX SoCs aren't the only >> ones >> with a watchdog, just the first one added to Xen that pings the watchdog >> over >> an SMC. What about the OMAPs, the R-Cars, Xilinx's, Exynos' and so on? > > As far as I know the watchdog is usually not active until a driver activates it. > Which means that by default it will not fire. > Having it activated by the bootloader by default is not usual. > Now definitely on a lot of use cases the users are activating it in the booloader > but their systems are design for it. > > Do I understand that the default boot loader configuration is not activating it on your side ? We are using a bootloader called Punchboot [1] developed by one of our employees. With Punchboot you have to explicitly write the code to enable the watchdog yourself. In u-boot you need to enable the watchdog drivers and configure the watchdog first before it is started. I don't know how the situation is in other bootloaders such as BareBox. Best regards // John Ernberg [1]: https://github.com/jonasblixt/punchboot
Hi John, > On 25 Mar 2024, at 13:18, John Ernberg <john.ernberg@actia.se> wrote: > > Hi Bertrand, > > On 3/21/24 17:15, Bertrand Marquis wrote: >> Hi John, >> >>> On 21 Mar 2024, at 17:05, John Ernberg <john.ernberg@actia.se> wrote: >>> >>> Hi Bertrand, >>> >>> On 3/21/24 08:41, Bertrand Marquis wrote: >>>> Hi, >>>> >>>>> On 20 Mar 2024, at 18:40, Julien Grall <julien@xen.org> wrote: >>>>> >>>>> Hi John, >>>>> >>>>> On 20/03/2024 16:24, John Ernberg wrote: >>>>>> Hi Bertrand, >>>>>> On 3/13/24 11:07, Bertrand Marquis wrote: >>>>>>> Hi, >>>>>>> >>>>>>>> On 8 Mar 2024, at 15:04, Julien Grall <julien@xen.org> wrote: >>>>>>>> >>>>>>>> Hi John, >>>>>>>> >>>>>>>> Thank you for the reply. >>>>>>>> >>>>>>>> On 08/03/2024 13:40, John Ernberg wrote: >>>>>>>>> On 3/7/24 00:07, Julien Grall wrote: >>>>>>>>>>> Ping on the watchdog discussion bits. >>>>>>>>>> >>>>>>>>>> Sorry for the late reply. >>>>>>>>>> >>>>>>>>>> On 06/03/2024 13:13, John Ernberg wrote: >>>>>>>>>>> On 2/9/24 14:14, John Ernberg wrote: >>>>>>>>>>>> >>>>>>>>>>>>> * IMX_SIP_TIMER_*: This seems to be related to the watchdog. >>>>>>>>>>>>> Shouldn't dom0 rely on the watchdog provided by Xen instead? So those >>>>>>>>>>>>> call will be used by Xen. >>>>>>>>>>>> >>>>>>>>>>>> That is indeed a watchdog SIP, and also for setting the SoC internal RTC >>>>>>>>>>>> if it is being used. >>>>>>>>>>>> >>>>>>>>>>>> I looked around if there was previous discussion and only really >>>>>>>>>>>> found [3]. >>>>>>>>>>>> Is the xen/xen/include/watchdog.h header meant to be for this kind of >>>>>>>>>>>> watchdog support or is that more for the VM watchdog? Looking at the x86 >>>>>>>>>>>> ACPI NMI watchdog it seems like the former, but I have never worked with >>>>>>>>>>>> x86 nor ACPI. >>>>>>>>>> >>>>>>>>>> include/watchdog.h contains helper to configure the watchdog for Xen. We >>>>>>>>>> also have per-VM watchdog and this is configured by the hypercall >>>>>>>>>> SCHEDOP_watchdog. >>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Currently forwarding it to Dom0 has not caused any watchdog resets with >>>>>>>>>>>> our watchdog timeout settings, our specific Dom0 setup and VM count. >>>>>>>>>> >>>>>>>>>> IIUC, the SMC API for the watchdog would be similar to the ACPI NMI >>>>>>>>>> watchdog. So I think it would make more sense if this is not exposed to >>>>>>>>>> dom0 (even if Xen is doing nothing with it). >>>>>>>>>> >>>>>>>>>> Can you try to hide the SMCs and check if dom0 still behave properly? >>>>>>>>>> >>>>>>>>>> Cheers, >>>>>>>>>> >>>>>>>>> This SMC manages a hardware watchdog, if it's not pinged within a >>>>>>>>> specific interval the entire board resets. >>>>>>>> >>>>>>>> Do you know what's the default interval? Is it large enough so Xen + dom0 can boot (at least up to when the watchdog driver is initialized)? >>>>>>>> >>>>>>>>> If I block the SMCs the watchdog driver in Dom0 will fail to ping the >>>>>>>>> watchdog, triggering a board reset because the system looks to have >>>>>>>>> become unresponsive. The reason this patch set started is because we >>>>>>>>> couldn't ping the watchdog when running with Xen. >>>>>>>>> In our specific system the bootloader enables the watchdog as early as >>>>>>>>> possible so that we can get watchdog protection for as much of the boot >>>>>>>>> as we possibly can. >>>>>>>>> So, if we are to block the SMC from Dom0, then Xen needs to take over >>>>>>>>> the pinging. It could be implemented similarly to the NMI watchdog, >>>>>>>>> except that the system will reset if the ping is missed rather than >>>>>>>>> backtrace. >>>>>>>>> It would also mean that Xen will get a whole watchdog driver-category >>>>>>>>> due to the watchdog being vendor and sometimes even SoC specific when it >>>>>>>>> comes to Arm. >>>>>>>>> My understanding of the domain watchdog code is that today the domain >>>>>>>>> needs to call SCHEDOP_watchdog at least once to start the watchdog >>>>>>>>> timer. Since watchdog protection through the whole boot process is >>>>>>>>> desirable we'd need some core changes, such as an option to start the >>>>>>>>> domain watchdog on init. > >>>>>>>>> It's quite a big change to make >>>>>>>> >>>>>>>> For clarification, above you seem to mention two changes: >>>>>>>> >>>>>>>> 1) Allow Xen to use the HW watchdog >>>>>>>> 2) Allow the domain to use the watchdog early >>>>>>>> >>>>>>>> I am assuming by big change, you are referring to 2? >>>>>>>> >>>>>>>> , while I am not against doing it if it >>>>>>>>> makes sense, I now wonder if Xen should manage hardware watchdogs. >>>>>>>>> Looking at the domain watchdog code it looks like if a domain does not >>>>>>>>> get enough execution time, the watchdog will not be pinged enough and >>>>>>>>> the guest will be reset. So either watchdog approach requires Dom0 to >>>>>>>>> get execution time. Dom0 also needs to service all the PV backends it's >>>>>>>>> responsible for. I'm not sure it's valuable to add another layer of >>>>>>>>> watchdog for this scenario as the end result (checking that the entire >>>>>>>>> system works) is achieved without it as well. >>>>>>>>> So, before I try to find the time to make a proposal for moving the >>>>>>>>> hardware watchdog bit to Xen, do we really want it? >>>>>>>> >>>>>>>> Thanks for the details. Given that the watchdog is enabled by the bootloader, I think we want Xen to drive the watchdog for two reasons: >>>>>>>> 1) In true dom0less environment, dom0 would not exist >>>>>>>> 2) You are relying on Xen + Dom0 to boot (or at least enough to get the watchdog working) within the watchdog interval. >>>>>>> >>>>>>> Definitely we need to consider the case where there is no Dom0. >>>>>>> >>>>>>> I think there are in fact 3 different use cases here: >>>>>>> - watchdog fully driven in a domain (dom0 or another): would work if it is expected >>>>>>> that Xen + Domain boot time is under the watchdog initial refresh rate. I think this >>>>>>> could make sense on some applications where your system depends on a specific >>>>>>> domain to be properly booted to work. This would require an initial refresh time >>>>>>> configurable in the boot loader probably. >>>>>> This is our use-case. ^ >>>>>> Our dom0 is monitoring and managing the other domains in our system. >>>>>> Without dom0 working the system isn't really working as a whole. >>>>>> @Julien: Would you be ok with the patch set continuing in the direction >>>>>> of the >>>>>> original proposal, letting another party (or me at a later time) implement >>>>>> the fully driven by Xen option? >>>>> I am concerned about this particular point from Bertrand: >>>>> >>>>> "would work if it is expected that Xen + Domain boot time is under the watchdog initial refresh rate." >>>>> >>>>> How will a user be able to figure out how to initially configure the watchdog? Is this something you can easily configure in the bootloader at runtime? >>> >>> If you go through the trouble of enabling the watchdog in the bootloader you >>> usually know what you're doing and set the timeout yourself. >>> >>> Since our systems can be mounted in really annoying locations (both in >>> installations and world-wise) we need as much auto-recovery as possible to >>> avoid people having to travel to collect a unit that just needed a reset due >>> to some unexpected obscure issue at startup. >> >> I definitely get the need do not get me wrong. >> I am just concerned by potential users having target restarting when using Xen because of that and not knowing why. >> >>>> >>>> Definitely here it would be better to have the watchdog turned off by default and document how to enable it in the firmware. >>>> >>>> Even if a long timeout is configured by default, a user could run into trouble if using a linux without watchdog or not running linux or using dom0less without a linux having access to it. >>>> I agree with Julien here that the concern could be that users would come to us instead of NXP if there is system is doing a reset without reasons after some seconds or minutes. >>> >>> I could add myself as a reviewer for the iMX parts if that helps routing >>> such >>> questions (and future patches) also to me. We have experience with the QXP, >>> and the QM (for the supported parts by this patch set) is identical. >>> >>> Would that help with the concerns? >> >> Definitely it could help. > > I'll figure out how to include myself in the MAINTAINERS file for the > next spin. > Thanks. >> >>> >>>> >>>>> >>>>> >>>>> Overall, I am not for this approach at least in the current status. I would be more inclined if there are some documentations explaining how this is supposed to be configured on NXP, so others can use the code. >>>>> >>>>> Anyway, this is why we have multiple Arm maintainers for this kind of situation. If they other agrees with you, then they can ack the patch and this can be merged. >>>> >>>> I agree with Stefano that it would be good to have those board supported. >>>> >>>> One thing i could suggest until there is a watchdog driver inside Xen is to have a clear Warning at Xen boot on those boards in the console so that we could at least identify the reason easily if a reset occurs for someone. >>> >>> How do other SoCs deal with this currently? The iMX SoCs aren't the only >>> ones >>> with a watchdog, just the first one added to Xen that pings the watchdog >>> over >>> an SMC. What about the OMAPs, the R-Cars, Xilinx's, Exynos' and so on? >> >> As far as I know the watchdog is usually not active until a driver activates it. >> Which means that by default it will not fire. >> Having it activated by the bootloader by default is not usual. >> Now definitely on a lot of use cases the users are activating it in the booloader >> but their systems are design for it. >> >> Do I understand that the default boot loader configuration is not activating it on your side ? > > We are using a bootloader called Punchboot [1] developed by one of our > employees. With Punchboot you have to explicitly write the code to > enable the watchdog yourself. In u-boot you need to enable the watchdog > drivers and configure the watchdog first before it is started. I don't > know how the situation is in other bootloaders such as BareBox. Then a user not knowing will not have it activated and will not encounter issues. So all good from my point of view :-) Regards Bertrand > > Best regards // John Ernberg > > [1]: https://github.com/jonasblixt/punchboot
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile index 8632f4115f..bec6e55d1f 100644 --- a/xen/arch/arm/platforms/Makefile +++ b/xen/arch/arm/platforms/Makefile @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o obj-$(CONFIG_ALL64_PLAT) += thunderx.o obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o diff --git a/xen/arch/arm/platforms/imx8qm.c b/xen/arch/arm/platforms/imx8qm.c new file mode 100644 index 0000000000..a9cd9c3615 --- /dev/null +++ b/xen/arch/arm/platforms/imx8qm.c @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * xen/arch/arm/platforms/imx8qm.c + * + * i.MX 8QM setup + * + * Copyright (c) 2016 Freescale Inc. + * Copyright 2018-2019 NXP + * + * + * Peng Fan <peng.fan@nxp.com> + */ + +#include <asm/platform.h> +#include <asm/smccc.h> + +static const char * const imx8qm_dt_compat[] __initconst = +{ + "fsl,imx8qm", + "fsl,imx8qxp", + NULL +}; + +static bool imx8qm_smc(struct cpu_user_regs *regs) +{ + struct arm_smccc_res res; + + if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) + { + printk_once(XENLOG_WARNING "no SMCCC 1.1 support. Disabling firmware calls\n"); + + return false; + } + + arm_smccc_1_1_smc(get_user_reg(regs, 0), + get_user_reg(regs, 1), + get_user_reg(regs, 2), + get_user_reg(regs, 3), + get_user_reg(regs, 4), + get_user_reg(regs, 5), + get_user_reg(regs, 6), + get_user_reg(regs, 7), + &res); + + set_user_reg(regs, 0, res.a0); + set_user_reg(regs, 1, res.a1); + set_user_reg(regs, 2, res.a2); + set_user_reg(regs, 3, res.a3); + + return true; +} + +PLATFORM_START(imx8qm, "i.MX 8") + .compatible = imx8qm_dt_compat, + .smc = imx8qm_smc, +PLATFORM_END + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */
When using Linux for dom0 there are a bunch of drivers that need to do SMC SIP calls into the PSCI provider to enable certain hardware bits like the watchdog. Provide a basic platform glue that implements the needed SMC forwarding. Signed-off-by: John Ernberg <john.ernberg@actia.se> --- NOTE: This is based on code found in NXP Xen tree located here: https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c xen/arch/arm/platforms/Makefile | 1 + xen/arch/arm/platforms/imx8qm.c | 65 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 xen/arch/arm/platforms/imx8qm.c