| Message ID | 20240408161129.900347-2-john.ernberg@actia.se (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Xen: ARM: Improved NXP iMX8 platform support | expand |
> Subject: [PATCH v4 1/3] xen/arm: Add imx8q{m,x} platform glue > > 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. > > The format of these calls are as follows: > - reg 0: function ID > - reg 1: subfunction ID (when there's a subfunction) remaining regs: args > > For now we only allow Dom0 to make these calls as they are all managing > hardware. There is no specification for these SIP calls, the IDs and names have > been extracted from the upstream linux kernel and the vendor kernel. > > Most of the SIP calls are only available for the iMX8M series of SoCs, so they > are easy to reject and they need to be revisited when iMX8M series support is > added. > > From the other calls we can reject CPUFREQ because Dom0 cannot make an > informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake > up from suspend, which Xen doesn't support at this time. > > This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, > which for now are allowed to Dom0. > > NOTE: This code is based on code found in NXP Xen tree located here: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub > .com%2Fnxp-imx%2Fimx-xen%2Fblob%2Flf- > 5.10.y_4.13%2Fxen%2Farch%2Farm%2Fplatforms%2Fimx8qm.c&data=05%7 > C02%7Cpeng.fan%40nxp.com%7C5602a2d54f074c7dcd9608dc57e69286%7 > C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6384818950076004 > 32%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=qSf9A7yE17P > 2q2%2FqXINORXh2EfFAS1%2BilhiY%2FexjcbA%3D&reserved=0 > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > [jernberg: Add SIP call filtering] > Signed-off-by: John Ernberg <john.ernberg@actia.se> > > --- > > v4: > - Fix coding style problems added in v3 (Michal Orzel) > - Remove all calls not currently used by Linux on imx8q{m,xp} (Michal Orzel) > - Fix {g,}printk inconsistencies, prefer gprintk (Michal Orzel) > > v3: > - Adhere to style guidelines for line length and label indentation (Michal > Orzel) > - Use smccc macros to build the SIP function identifier (Michal Orzel) > - Adjust platform name to be specific to QM and QXP variants (Michal Orzel) > - Pick up additional SIPs which may be used by other Linux versions (Michal > Orzel) > - Changes to the commit message due to above > > v2: > - Reword the commit message to be a bit clearer > - Include the link previously added as a context note to the commit message > (Julien Grall) > - Add Pengs signed off (Julien Grall, Peng Fan) > - Add basic SIP call filter (Julien Grall) > - Expand the commit message a whole bunch because of the changes to the > code > --- > xen/arch/arm/platforms/Makefile | 1 + > xen/arch/arm/platforms/imx8qm.c | 139 > ++++++++++++++++++++++++++++++++ > 2 files changed, 140 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..3600a073e8 > --- /dev/null > +++ b/xen/arch/arm/platforms/imx8qm.c > @@ -0,0 +1,139 @@ > +/* 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 <xen/sched.h> > +#include <asm/platform.h> > +#include <asm/smccc.h> > + > +static const char * const imx8qm_dt_compat[] __initconst = { > + "fsl,imx8qm", > + "fsl,imx8qxp", > + NULL > +}; > + > +#define IMX_SIP_FID(fid) \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_CONV_64, \ > + ARM_SMCCC_OWNER_SIP, \ > + (fid)) > + > +#define IMX_SIP_F_CPUFREQ 0x1 > +#define IMX_SIP_F_TIME 0x2 > +#define IMX_SIP_F_WAKEUP_SRC 0x9 > +#define IMX_SIP_F_OTP_WRITE 0xB > + > +#define IMX_SIP_TIME_SF_RTC_SET_TIME 0x00 > +#define IMX_SIP_TIME_SF_WDOG_START 0x01 > +#define IMX_SIP_TIME_SF_WDOG_STOP 0x02 > +#define IMX_SIP_TIME_SF_WDOG_SET_ACT 0x03 > +#define IMX_SIP_TIME_SF_WDOG_PING 0x04 > +#define IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT 0x05 > +#define IMX_SIP_TIME_SF_WDOG_GET_STAT 0x06 > +#define IMX_SIP_TIME_SF_WDOG_SET_PRETIME 0x07 > + > +static bool imx8qm_is_sip_time_call_ok(uint32_t subfunction_id) { > + switch ( subfunction_id ) > + { > + case IMX_SIP_TIME_SF_RTC_SET_TIME: > + return true; > + case IMX_SIP_TIME_SF_WDOG_START: > + case IMX_SIP_TIME_SF_WDOG_STOP: > + case IMX_SIP_TIME_SF_WDOG_SET_ACT: > + case IMX_SIP_TIME_SF_WDOG_PING: > + case IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT: > + case IMX_SIP_TIME_SF_WDOG_GET_STAT: > + case IMX_SIP_TIME_SF_WDOG_SET_PRETIME: > + return true; > + default: > + gprintk(XENLOG_WARNING, "imx8qm: smc: time: Unknown > subfunction id %x\n", > + subfunction_id); > + return false; > + } > +} > + > +static bool imx8qm_smc(struct cpu_user_regs *regs) { > + uint32_t function_id = get_user_reg(regs, 0); > + uint32_t subfunction_id = get_user_reg(regs, 1); > + struct arm_smccc_res res; > + > + if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) > + { > + printk_once(XENLOG_WARNING > + "imx8qm: smc: no SMCCC 1.1 support. Disabling > + firmware calls\n"); > + > + return false; > + } > + > + /* Only hardware domain may use the SIP calls */ > + if ( !is_hardware_domain(current->domain) ) > + { > + gprintk(XENLOG_WARNING, "imx8qm: smc: No access\n"); > + return false; > + } > + > + switch ( function_id ) > + { > + case IMX_SIP_FID(IMX_SIP_F_CPUFREQ): > + /* Hardware domain can't take any informed decision here */ > + return false; > + case IMX_SIP_FID(IMX_SIP_F_TIME): > + if ( imx8qm_is_sip_time_call_ok(subfunction_id) ) > + goto allow_call; > + return false; > + /* Xen doesn't have suspend support */ > + case IMX_SIP_FID(IMX_SIP_F_WAKEUP_SRC): > + return false; > + case IMX_SIP_FID(IMX_SIP_F_OTP_WRITE): > + /* subfunction_id is the fuse number, no sensible check possible */ > + goto allow_call; > + default: > + gprintk(XENLOG_WARNING, "imx8qm: smc: Unknown function > id %x\n", > + function_id); > + return false; > + } > + > + allow_call: > + arm_smccc_1_1_smc(function_id, > + subfunction_id, > + 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 8Q{M,XP}") > + .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: > + */ > -- Reviewed-by: Peng Fan <peng.fan@nxp.com>
Hi John, On 08/04/2024 18:11, John Ernberg wrote: > > > 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. > > The format of these calls are as follows: > - reg 0: function ID > - reg 1: subfunction ID (when there's a subfunction) > remaining regs: args > > For now we only allow Dom0 to make these calls as they are all managing > hardware. There is no specification for these SIP calls, the IDs and names > have been extracted from the upstream linux kernel and the vendor kernel. > > Most of the SIP calls are only available for the iMX8M series of SoCs, so > they are easy to reject and they need to be revisited when iMX8M series > support is added. Stale paragraph. Should be removed given that the driver targets only Q{M,XP}. > > From the other calls we can reject CPUFREQ because Dom0 cannot make an > informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake > up from suspend, which Xen doesn't support at this time. > > This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which > for now are allowed to Dom0. BUILDINFO, TEMP ALARM are leftovers from previous revision. > > NOTE: This code 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 > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > [jernberg: Add SIP call filtering] > Signed-off-by: John Ernberg <john.ernberg@actia.se> Reviewed-by: Michal Orzel <michal.orzel@amd.com> The commit msg can be fixed on commit. ~Michal
On 4/9/24 8:47 AM, Michal Orzel wrote: > Hi John, > > On 08/04/2024 18:11, John Ernberg wrote: >> >> >> 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. >> >> The format of these calls are as follows: >> - reg 0: function ID >> - reg 1: subfunction ID (when there's a subfunction) >> remaining regs: args >> >> For now we only allow Dom0 to make these calls as they are all managing >> hardware. There is no specification for these SIP calls, the IDs and names >> have been extracted from the upstream linux kernel and the vendor kernel. >> >> Most of the SIP calls are only available for the iMX8M series of SoCs, so >> they are easy to reject and they need to be revisited when iMX8M series >> support is added. > Stale paragraph. Should be removed given that the driver targets only Q{M,XP}. > >> >> From the other calls we can reject CPUFREQ because Dom0 cannot make an >> informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake >> up from suspend, which Xen doesn't support at this time. >> >> This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which >> for now are allowed to Dom0. > BUILDINFO, TEMP ALARM are leftovers from previous revision. > >> >> NOTE: This code 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 >> >> Signed-off-by: Peng Fan <peng.fan@nxp.com> >> [jernberg: Add SIP call filtering] >> Signed-off-by: John Ernberg <john.ernberg@actia.se> > Reviewed-by: Michal Orzel <michal.orzel@amd.com> > > The commit msg can be fixed on commit. > > ~Michal Apologies for forgetting to adjust that. Let me know if it's easier for you if I do a v5 with the fixed commit message. Thanks! // John Ernberg
Hi John, On 10/04/2024 08:53, John Ernberg wrote: > > > On 4/9/24 8:47 AM, Michal Orzel wrote: >> Hi John, >> >> On 08/04/2024 18:11, John Ernberg wrote: >>> >>> >>> 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. >>> >>> The format of these calls are as follows: >>> - reg 0: function ID >>> - reg 1: subfunction ID (when there's a subfunction) >>> remaining regs: args >>> >>> For now we only allow Dom0 to make these calls as they are all managing >>> hardware. There is no specification for these SIP calls, the IDs and names >>> have been extracted from the upstream linux kernel and the vendor kernel. >>> >>> Most of the SIP calls are only available for the iMX8M series of SoCs, so >>> they are easy to reject and they need to be revisited when iMX8M series >>> support is added. >> Stale paragraph. Should be removed given that the driver targets only Q{M,XP}. >> >>> >>> From the other calls we can reject CPUFREQ because Dom0 cannot make an >>> informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake >>> up from suspend, which Xen doesn't support at this time. >>> >>> This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which >>> for now are allowed to Dom0. >> BUILDINFO, TEMP ALARM are leftovers from previous revision. >> >>> >>> NOTE: This code 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 >>> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com> >>> [jernberg: Add SIP call filtering] >>> Signed-off-by: John Ernberg <john.ernberg@actia.se> >> Reviewed-by: Michal Orzel <michal.orzel@amd.com> >> >> The commit msg can be fixed on commit. >> >> ~Michal > > > Apologies for forgetting to adjust that. Let me know if it's easier for > you if I do a v5 with the fixed commit message. > > Thanks! // John Ernberg The series is already committed and Stefano fixed my remarks on commit. ~Michal
Hi John, I saw this patch was committed. I have one question this may require some adjustment. On 08/04/2024 17:11, John Ernberg wrote: > --- > xen/arch/arm/platforms/Makefile | 1 + > xen/arch/arm/platforms/imx8qm.c | 139 ++++++++++++++++++++++++++++++++ > 2 files changed, 140 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..3600a073e8 > --- /dev/null > +++ b/xen/arch/arm/platforms/imx8qm.c > @@ -0,0 +1,139 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ The majority of Xen code is using GPL-2.0-only. In the early days for Xen on Arm we started to use GPLv2+ which I consider it was a mistake. Unfortunately this started to spread as people copied/pasted the same copyright headers. So can you confirm whether you intended to use GPL-2.0+? If not would you be able to send a patch to adjust it? (Better to it before there are more modifications). Cheers,
On 15/04/2024 11:25 am, Julien Grall wrote: > Hi John, > > I saw this patch was committed. I have one question this may require > some adjustment. > > On 08/04/2024 17:11, John Ernberg wrote: >> --- >> xen/arch/arm/platforms/Makefile | 1 + >> xen/arch/arm/platforms/imx8qm.c | 139 ++++++++++++++++++++++++++++++++ >> 2 files changed, 140 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..3600a073e8 >> --- /dev/null >> +++ b/xen/arch/arm/platforms/imx8qm.c >> @@ -0,0 +1,139 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > The majority of Xen code is using GPL-2.0-only. In the early days for > Xen on Arm we started to use GPLv2+ which I consider it was a mistake. > Unfortunately this started to spread as people copied/pasted the same > copyright headers. > > So can you confirm whether you intended to use GPL-2.0+? If not would > you be able to send a patch to adjust it? (Better to it before there > are more modifications). Julien: I've called you out multiple times before. Don't ever bully contributors into changing licensing. It is unacceptable behaviour, and in most cases - including this one by the looks of things - not legal. John: Thankyou for your contribution. It has been made under a license compatible with the rest of Xen. There is no need to make any change, and please do not feel pressured into doing so. ~Andrew
On 15/04/2024 11:50, Andrew Cooper wrote: > On 15/04/2024 11:25 am, Julien Grall wrote: >> Hi John, >> >> I saw this patch was committed. I have one question this may require >> some adjustment. >> >> On 08/04/2024 17:11, John Ernberg wrote: >>> --- >>> xen/arch/arm/platforms/Makefile | 1 + >>> xen/arch/arm/platforms/imx8qm.c | 139 ++++++++++++++++++++++++++++++++ >>> 2 files changed, 140 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..3600a073e8 >>> --- /dev/null >>> +++ b/xen/arch/arm/platforms/imx8qm.c >>> @@ -0,0 +1,139 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> >> The majority of Xen code is using GPL-2.0-only. In the early days for >> Xen on Arm we started to use GPLv2+ which I consider it was a mistake. >> Unfortunately this started to spread as people copied/pasted the same >> copyright headers. >> >> So can you confirm whether you intended to use GPL-2.0+? If not would >> you be able to send a patch to adjust it? (Better to it before there >> are more modifications). > > Julien: I've called you out multiple times before. And there are multiple thread explaining why I am requesting if we can use GPLv2-only. In fact from CONTRIBUTING: The recommended license of a directory will depend on the COPYING file. If the new file is using a different license, this should be highlighted and discussed in the commit message or cover letter introducing the file. > Don't ever bully contributors into changing licensing. It is > unacceptable behaviour, and in most cases - including this one by the > looks of things - not legal. I don't think I have bullied the contributor. I have asked politely whether it can be done. There is nothing illegal (see above). The problematic behavior is you trying to pressure the other people to accept your point of view by been condescending or insulting them like you did here. I have reported this behavior several times to CoC. And I guess this need to happen again. Cheers,
Hi Julien, On 4/15/24 1:03 PM, Julien Grall wrote: > > > On 15/04/2024 11:50, Andrew Cooper wrote: >> On 15/04/2024 11:25 am, Julien Grall wrote: >>> Hi John, >>> >>> I saw this patch was committed. I have one question this may require >>> some adjustment. >>> >>> On 08/04/2024 17:11, John Ernberg wrote: >>>> --- >>>> xen/arch/arm/platforms/Makefile | 1 + >>>> xen/arch/arm/platforms/imx8qm.c | 139 >>>> ++++++++++++++++++++++++++++++++ >>>> 2 files changed, 140 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..3600a073e8 >>>> --- /dev/null >>>> +++ b/xen/arch/arm/platforms/imx8qm.c >>>> @@ -0,0 +1,139 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >>> >>> The majority of Xen code is using GPL-2.0-only. In the early days for >>> Xen on Arm we started to use GPLv2+ which I consider it was a mistake. >>> Unfortunately this started to spread as people copied/pasted the same >>> copyright headers. >>> >>> So can you confirm whether you intended to use GPL-2.0+? If not would >>> you be able to send a patch to adjust it? (Better to it before there >>> are more modifications). >> >> Julien: I've called you out multiple times before. > > And there are multiple thread explaining why I am requesting if we can > use GPLv2-only. In fact from CONTRIBUTING: > > The recommended license of a directory will depend on the COPYING file. > If the new file is using a different license, this should be highlighted > and discussed in the commit message or cover letter introducing the > file. > Since part of the code was not written by me, but by Peng, I think both of us need to agree to a license change if one is to be made. I am personally fine with either license. >> Don't ever bully contributors into changing licensing. It is >> unacceptable behaviour, and in most cases - including this one by the >> looks of things - not legal. > > I don't think I have bullied the contributor. I have asked politely > whether it can be done. There is nothing illegal (see above). Just adding: I did not feel bullied here. > > The problematic behavior is you trying to pressure the other people to > accept your point of view by been condescending or insulting them like > you did here. > > I have reported this behavior several times to CoC. And I guess this > need to happen again. > > Cheers, > Best regards // John Ernberg
Hi everyone, We all have a responsibility and pledge to make this community a welcoming environment. As such, I would like to request we keep the Code of Conduct and patch discussions separate. Should there be a need for any Code of Conduct complaints/investigations, this will be treated separately. Thank you John for your contributions. Many thanks, Kelly Choi Community Manager Xen Project On Mon, Apr 15, 2024 at 12:03 PM Julien Grall <julien@xen.org> wrote: > > > On 15/04/2024 11:50, Andrew Cooper wrote: > > On 15/04/2024 11:25 am, Julien Grall wrote: > >> Hi John, > >> > >> I saw this patch was committed. I have one question this may require > >> some adjustment. > >> > >> On 08/04/2024 17:11, John Ernberg wrote: > >>> --- > >>> xen/arch/arm/platforms/Makefile | 1 + > >>> xen/arch/arm/platforms/imx8qm.c | 139 > ++++++++++++++++++++++++++++++++ > >>> 2 files changed, 140 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..3600a073e8 > >>> --- /dev/null > >>> +++ b/xen/arch/arm/platforms/imx8qm.c > >>> @@ -0,0 +1,139 @@ > >>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ > >> > >> The majority of Xen code is using GPL-2.0-only. In the early days for > >> Xen on Arm we started to use GPLv2+ which I consider it was a mistake. > >> Unfortunately this started to spread as people copied/pasted the same > >> copyright headers. > >> > >> So can you confirm whether you intended to use GPL-2.0+? If not would > >> you be able to send a patch to adjust it? (Better to it before there > >> are more modifications). > > > > Julien: I've called you out multiple times before. > > And there are multiple thread explaining why I am requesting if we can > use GPLv2-only. In fact from CONTRIBUTING: > > The recommended license of a directory will depend on the COPYING file. > If the new file is using a different license, this should be highlighted > and discussed in the commit message or cover letter introducing the > file. > > > Don't ever bully contributors into changing licensing. It is > > unacceptable behaviour, and in most cases - including this one by the > > looks of things - not legal. > > I don't think I have bullied the contributor. I have asked politely > whether it can be done. There is nothing illegal (see above). > > The problematic behavior is you trying to pressure the other people to > accept your point of view by been condescending or insulting them like > you did here. > > I have reported this behavior several times to CoC. And I guess this > need to happen again. > > Cheers, > > -- > Julien Grall >
Hi John, On 15/04/2024 12:17, John Ernberg wrote: > Hi Julien, > > On 4/15/24 1:03 PM, Julien Grall wrote: >> >> >> On 15/04/2024 11:50, Andrew Cooper wrote: >>> On 15/04/2024 11:25 am, Julien Grall wrote: >>>> Hi John, >>>> >>>> I saw this patch was committed. I have one question this may require >>>> some adjustment. >>>> >>>> On 08/04/2024 17:11, John Ernberg wrote: >>>>> --- >>>>> xen/arch/arm/platforms/Makefile | 1 + >>>>> xen/arch/arm/platforms/imx8qm.c | 139 >>>>> ++++++++++++++++++++++++++++++++ >>>>> 2 files changed, 140 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..3600a073e8 >>>>> --- /dev/null >>>>> +++ b/xen/arch/arm/platforms/imx8qm.c >>>>> @@ -0,0 +1,139 @@ >>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >>>> >>>> The majority of Xen code is using GPL-2.0-only. In the early days for >>>> Xen on Arm we started to use GPLv2+ which I consider it was a mistake. >>>> Unfortunately this started to spread as people copied/pasted the same >>>> copyright headers. >>>> >>>> So can you confirm whether you intended to use GPL-2.0+? If not would >>>> you be able to send a patch to adjust it? (Better to it before there >>>> are more modifications). >>> >>> Julien: I've called you out multiple times before. >> >> And there are multiple thread explaining why I am requesting if we can >> use GPLv2-only. In fact from CONTRIBUTING: >> >> The recommended license of a directory will depend on the COPYING file. >> If the new file is using a different license, this should be highlighted >> and discussed in the commit message or cover letter introducing the >> file. >> > > Since part of the code was not written by me, but by Peng, I think both > of us need to agree to a license change if one is to be made. Ah I didn't realize that Peng also contributed. Let's wait if he is happy with the change. Also, offline, I was pointed out that I could explain a little bit more why I asked if this could be changed. From [1]: "IIRC from past discussion there are two broads concern with GPLv2+: - We are leaving the choice of which license applies to the person copying the code. So if a new version is released that is less favorable to the initial contributor, then we have no leverage. - Some companies are rather cautious to contribute code that may be licensed under GPLv3 (would be allowed with GPLv2+). The later is particularly a problem because not many people realize that a fair part of Xen on Arm is GPLv2+. I never really understood why we chose that (this was before my time) but this got spread as the existing copyright was added to a new file. Admittely, the contributor should be more cautious. But I would not say this is trivial to spot the difference." Cheers, [1] https://lore.kernel.org/xen-devel/f235f6f8-d585-4e24-7fc8-3f2df9240c9d@xen.org/
Hi Julien, John, > Subject: Re: [PATCH v4 1/3] xen/arm: Add imx8q{m,x} platform glue > > Hi John, > > On 15/04/2024 12:17, John Ernberg wrote: > > Hi Julien, > > > > On 4/15/24 1:03 PM, Julien Grall wrote: > >> > >> > >> On 15/04/2024 11:50, Andrew Cooper wrote: > >>> On 15/04/2024 11:25 am, Julien Grall wrote: > >>>> Hi John, > >>>> > >>>> I saw this patch was committed. I have one question this may > >>>> require some adjustment. > >>>> > >>>> On 08/04/2024 17:11, John Ernberg wrote: > >>>>> --- > >>>>> xen/arch/arm/platforms/Makefile | 1 + > >>>>> xen/arch/arm/platforms/imx8qm.c | 139 > >>>>> ++++++++++++++++++++++++++++++++ > >>>>> 2 files changed, 140 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..3600a073e8 > >>>>> --- /dev/null > >>>>> +++ b/xen/arch/arm/platforms/imx8qm.c > >>>>> @@ -0,0 +1,139 @@ > >>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */ > >>>> > >>>> The majority of Xen code is using GPL-2.0-only. In the early days > >>>> for Xen on Arm we started to use GPLv2+ which I consider it was a > mistake. > >>>> Unfortunately this started to spread as people copied/pasted the > >>>> same copyright headers. > >>>> > >>>> So can you confirm whether you intended to use GPL-2.0+? If not > >>>> would you be able to send a patch to adjust it? (Better to it > >>>> before there are more modifications). > >>> > >>> Julien: I've called you out multiple times before. > >> > >> And there are multiple thread explaining why I am requesting if we > >> can use GPLv2-only. In fact from CONTRIBUTING: > >> > >> The recommended license of a directory will depend on the COPYING file. > >> If the new file is using a different license, this should be > >> highlighted and discussed in the commit message or cover letter > >> introducing the file. > >> > > > > Since part of the code was not written by me, but by Peng, I think > > both of us need to agree to a license change if one is to be made. > > Ah I didn't realize that Peng also contributed. Let's wait if he is happy with the > change. I am ok with the change. Thanks, Peng. > > Also, offline, I was pointed out that I could explain a little bit more why I > asked if this could be changed. From [1]: > > "IIRC from past discussion there are two broads concern with GPLv2+: > - We are leaving the choice of which license applies to the person copying > the code. So if a new version is released that is less favorable to the initial > contributor, then we have no leverage. > - Some companies are rather cautious to contribute code that may be > licensed under GPLv3 (would be allowed with GPLv2+). > > The later is particularly a problem because not many people realize that a fair > part of Xen on Arm is GPLv2+. I never really understood why we chose that > (this was before my time) but this got spread as the existing copyright was > added to a new file. Admittely, the contributor should be more cautious. But I > would not say this is trivial to spot the difference." > > Cheers, > > [1] > https://lore.ke/ > rnel.org%2Fxen-devel%2Ff235f6f8-d585-4e24-7fc8- > 3f2df9240c9d%40xen.org%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C > 07ac104b960d476c349f08dc5d420db6%7C686ea1d3bc2b4c6fa92cd99c5c3 > 01635%7C0%7C0%7C638487785488861385%7CUnknown%7CTWFpbGZsb3 > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0 > %3D%7C0%7C%7C%7C&sdata=d5RQJ8nek5a8Xgi%2B%2F9g3GqDL8CZmd3c > O9EN6w57S9qE%3D&reserved=0 > > -- > Julien Grall
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..3600a073e8 --- /dev/null +++ b/xen/arch/arm/platforms/imx8qm.c @@ -0,0 +1,139 @@ +/* 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 <xen/sched.h> +#include <asm/platform.h> +#include <asm/smccc.h> + +static const char * const imx8qm_dt_compat[] __initconst = +{ + "fsl,imx8qm", + "fsl,imx8qxp", + NULL +}; + +#define IMX_SIP_FID(fid) \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_CONV_64, \ + ARM_SMCCC_OWNER_SIP, \ + (fid)) + +#define IMX_SIP_F_CPUFREQ 0x1 +#define IMX_SIP_F_TIME 0x2 +#define IMX_SIP_F_WAKEUP_SRC 0x9 +#define IMX_SIP_F_OTP_WRITE 0xB + +#define IMX_SIP_TIME_SF_RTC_SET_TIME 0x00 +#define IMX_SIP_TIME_SF_WDOG_START 0x01 +#define IMX_SIP_TIME_SF_WDOG_STOP 0x02 +#define IMX_SIP_TIME_SF_WDOG_SET_ACT 0x03 +#define IMX_SIP_TIME_SF_WDOG_PING 0x04 +#define IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT 0x05 +#define IMX_SIP_TIME_SF_WDOG_GET_STAT 0x06 +#define IMX_SIP_TIME_SF_WDOG_SET_PRETIME 0x07 + +static bool imx8qm_is_sip_time_call_ok(uint32_t subfunction_id) +{ + switch ( subfunction_id ) + { + case IMX_SIP_TIME_SF_RTC_SET_TIME: + return true; + case IMX_SIP_TIME_SF_WDOG_START: + case IMX_SIP_TIME_SF_WDOG_STOP: + case IMX_SIP_TIME_SF_WDOG_SET_ACT: + case IMX_SIP_TIME_SF_WDOG_PING: + case IMX_SIP_TIME_SF_WDOG_SET_TIMEOUT: + case IMX_SIP_TIME_SF_WDOG_GET_STAT: + case IMX_SIP_TIME_SF_WDOG_SET_PRETIME: + return true; + default: + gprintk(XENLOG_WARNING, "imx8qm: smc: time: Unknown subfunction id %x\n", + subfunction_id); + return false; + } +} + +static bool imx8qm_smc(struct cpu_user_regs *regs) +{ + uint32_t function_id = get_user_reg(regs, 0); + uint32_t subfunction_id = get_user_reg(regs, 1); + struct arm_smccc_res res; + + if ( !cpus_have_const_cap(ARM_SMCCC_1_1) ) + { + printk_once(XENLOG_WARNING + "imx8qm: smc: no SMCCC 1.1 support. Disabling firmware calls\n"); + + return false; + } + + /* Only hardware domain may use the SIP calls */ + if ( !is_hardware_domain(current->domain) ) + { + gprintk(XENLOG_WARNING, "imx8qm: smc: No access\n"); + return false; + } + + switch ( function_id ) + { + case IMX_SIP_FID(IMX_SIP_F_CPUFREQ): + /* Hardware domain can't take any informed decision here */ + return false; + case IMX_SIP_FID(IMX_SIP_F_TIME): + if ( imx8qm_is_sip_time_call_ok(subfunction_id) ) + goto allow_call; + return false; + /* Xen doesn't have suspend support */ + case IMX_SIP_FID(IMX_SIP_F_WAKEUP_SRC): + return false; + case IMX_SIP_FID(IMX_SIP_F_OTP_WRITE): + /* subfunction_id is the fuse number, no sensible check possible */ + goto allow_call; + default: + gprintk(XENLOG_WARNING, "imx8qm: smc: Unknown function id %x\n", + function_id); + return false; + } + + allow_call: + arm_smccc_1_1_smc(function_id, + subfunction_id, + 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 8Q{M,XP}") + .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: + */