Hi Julien, Stefano, On 01/10/2024 13:01, Julien Grall wrote: > Hi Andrei, > > On 30/09/2024 12:47, Andrei Cherechesu (OSS) wrote: >> From: Andrei Cherechesu <andrei.cherech...@nxp.com> >> >> Add code for NXP S32CC platforms (S32G2, S32G3, S32R45). >> >> S32CC platforms use the NXP LINFlexD UART driver for >> console by default, and rely on Dom0 having access to >> SCMI services for system-level resources from firmware >> at EL3. >> >> Platform-related quirks will be addressed in following >> commits. > > I don't see any specific quirks in the series. Are you intended to send > follow-up? > > [...]
Well, we could use an opinion on some SoC erratum implementation and if it would be acceptable for you or not, from any of the ARM maintainers. The erratum is about some TLB Invalidate (tlbi) instruction variants (by Virtual Address) which are handled incorrectly in some cases (for some VAs), and that need to be replaced with broader versions. It doesn't apply for all S32CC platforms, but we can use the Host DT compatible to identify this. My idea is that we define a platform quirk for this and check for it using platform_has_quirk(). Then, we either: - modify the definition for the affected 'tlbi' variant in arm64/flushtlb.h to actually execute the broader one if needed or: - define a new wrapper for the tlbi variant we want to replace the affected one with - check for this quirk before its usage and call the more potent version if needed (there's only one occurrence of usage for the affected version). Number 2 seems better to me, it's more customizable and it's similar to the existing handling for PLATFORM_QUIRK_GIC_64K_STRIDE for some other Arm platform. Do you have any suggestion/preference regarding how this should be implemented? > > >> diff --git a/xen/arch/arm/platforms/Makefile >> b/xen/arch/arm/platforms/Makefile >> index bec6e55d1f..2c304b964d 100644 >> --- a/xen/arch/arm/platforms/Makefile >> +++ b/xen/arch/arm/platforms/Makefile >> @@ -10,5 +10,6 @@ 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_S32CC_PLATFORM) += s32cc.o >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o >> diff --git a/xen/arch/arm/platforms/s32cc.c b/xen/arch/arm/platforms/s32cc.c >> new file mode 100644 >> index 0000000000..f99a2d56f6 >> --- /dev/null >> +++ b/xen/arch/arm/platforms/s32cc.c > > We only add a new platform if there are platform specific hook to implement. > AFAICT, you don't implement any, so it should not be necessary. Like I mentioned above, there's some erratum workaround which could make use of the platform_quirk() callback, that we want to send in the near future. > > Cheers, > Thank you for the review and support once again, Regards, Andrei C
