| Message ID | 832955382caa4ced744ec7894282592b62e8cf61.1651570561.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Spectre BHB follow up | expand |
Hi Bertrand, On 03/05/2022 10:38, Bertrand Marquis wrote: > Sync arm64 sysreg bit shift definitions with status of Linux kernel as > of 5.18-rc3 version (linux commit b2d229d4ddb1). > Sync ID registers sanitization with the status of Linux 5.18-rc3 and add > sanitization of ISAR2 registers. Please outline which specific commits you are actually backported. This would help to know what changed, why and also keep track of the autorships. When possible, the changes should be separated to match each Linux commit we backport. > Complete AA64ISAR2 and AA64MMFR1 with more fields. > While there add a comment for MMFR bitfields as for other registers in > the cpuinfo structure definition. AFAICT, this patch is doing 3 different things that are somewhat related: - Sync cpufeature.c - Update the headers with unused defines - Complete the structure cpufeature.h All those changes seem to be independent, so I think they should be done separately. This would help to keep the authorship right (your code vs Linux code). > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > --- > xen/arch/arm/arm64/cpufeature.c | 18 +++++- > xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- > xen/arch/arm/include/asm/cpufeature.h | 14 ++++- > 3 files changed, 91 insertions(+), 17 deletions(-) > > diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c > index 6e5d30dc7b..d9039d37b2 100644 > --- a/xen/arch/arm/arm64/cpufeature.c > +++ b/xen/arch/arm/arm64/cpufeature.c > @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = { > ARM64_FTR_END, > }; > > +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { > + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), > + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), > + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), > + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), So we are using CONFIG_ARM64_PTR_AUTH. But this is not defined in Kconfig. I realize there are more in cpufeature.c (somehow I didn't spot during preview), but I don't think this is right to define CONFIG_* without an associated entry in Kconfig. In one hand, I think it would be odd to add an entry in Kconfig because Xen wouldn't properly work if selected. On the other hand, it is useful if when we will implement pointer authentification. So maybe we should just add the Kconfig entry with a comment explaning why they are not selected. Any thoughts? Cheers,
Hi Julien, > On 3 May 2022, at 19:08, Julien Grall <julien@xen.org> wrote: > > Hi Bertrand, > > On 03/05/2022 10:38, Bertrand Marquis wrote: >> Sync arm64 sysreg bit shift definitions with status of Linux kernel as >> of 5.18-rc3 version (linux commit b2d229d4ddb1). >> Sync ID registers sanitization with the status of Linux 5.18-rc3 and add >> sanitization of ISAR2 registers. > Please outline which specific commits you are actually backported. This would help to know what changed, why and also keep track of the autorships. > > When possible, the changes should be separated to match each Linux commit we backport. As those are exactly identical to the linux tree, one can easily use git blame on the linux source tree to find those information if it is needed. I checked a bit and this is not something that was required before (for example when the cpufeature was introduced). > >> Complete AA64ISAR2 and AA64MMFR1 with more fields. >> While there add a comment for MMFR bitfields as for other registers in >> the cpuinfo structure definition. > > AFAICT, this patch is doing 3 different things that are somewhat related: > - Sync cpufeature.c > - Update the headers with unused defines > - Complete the structure cpufeature.h > > All those changes seem to be independent, so I think they should be done separately. This would help to keep the authorship right (your code vs Linux code). This and the previous request to split using linux commit will actually end up in 10 patches or more. In the current, the change can easily be checked doing a diff with the mentioned Linux version, so I am not really thrilled to make it more complex. Please confirm that all this is really what you want. > >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >> --- >> xen/arch/arm/arm64/cpufeature.c | 18 +++++- >> xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- >> xen/arch/arm/include/asm/cpufeature.h | 14 ++++- >> 3 files changed, 91 insertions(+), 17 deletions(-) >> diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c >> index 6e5d30dc7b..d9039d37b2 100644 >> --- a/xen/arch/arm/arm64/cpufeature.c >> +++ b/xen/arch/arm/arm64/cpufeature.c >> @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = { >> ARM64_FTR_END, >> }; >> +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >> + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), > So we are using CONFIG_ARM64_PTR_AUTH. But this is not defined in Kconfig. I realize there are more in cpufeature.c (somehow I didn't spot during preview), but I don't think this is right to define CONFIG_* without an associated entry in Kconfig. > > In one hand, I think it would be odd to add an entry in Kconfig because Xen wouldn't properly work if selected. On the other hand, it is useful if when we will implement pointer authentification. > > So maybe we should just add the Kconfig entry with a comment explaning why they are not selected. Any thoughts? This is really right and a very good catch. I think it would make sense to introduce those in Kconfig in order to keep the code equivalent to Linux. So I would suggest here to add hidden entries like this: ARM64_PTR_AUTH def_bool n depends on ARM64 help Pointer authentication support. This feature is not supported by Xen. Cheers Bertrand > > Cheers, > > -- > Julien Grall
On 04/05/2022 08:39, Bertrand Marquis wrote: > Hi Julien, Hi, >> On 3 May 2022, at 19:08, Julien Grall <julien@xen.org> wrote: >> >> Hi Bertrand, >> >> On 03/05/2022 10:38, Bertrand Marquis wrote: >>> Sync arm64 sysreg bit shift definitions with status of Linux kernel as >>> of 5.18-rc3 version (linux commit b2d229d4ddb1). >>> Sync ID registers sanitization with the status of Linux 5.18-rc3 and add >>> sanitization of ISAR2 registers. >> Please outline which specific commits you are actually backported. This would help to know what changed, why and also keep track of the autorships. >> >> When possible, the changes should be separated to match each Linux commit we backport. > > As those are exactly identical to the linux tree, one can easily use git blame on the linux source tree to find those information if it is needed Well... that's possible at the cost of everyone going through Linux to understand why the changes were made. This is not very scalable. > > I checked a bit and this is not something that was required before (for example when the cpufeature was introduced). If we import the full file, then we will generally don't log all the commits. However, for smaller changes, we will always mention the commit backported. There are several examples on the ML: - 0435784cc75d ("xen/arm: smmuv1: Intelligent SMR allocation") - 9c432b876bf5 ("x86/mwait-idle: add SPR support") We also recently introduced a tag "Origin:" to keep track of which commit was backported. If you want to understand the rationale, you can read this long thread: https://lore.kernel.org/xen-devel/0ed245fa-58a7-a5f6-b82e-48f9ed0b6970@suse.com/ > >> >>> Complete AA64ISAR2 and AA64MMFR1 with more fields. >>> While there add a comment for MMFR bitfields as for other registers in >>> the cpuinfo structure definition. >> >> AFAICT, this patch is doing 3 different things that are somewhat related: >> - Sync cpufeature.c >> - Update the headers with unused defines >> - Complete the structure cpufeature.h >> >> All those changes seem to be independent, so I think they should be done separately. This would help to keep the authorship right (your code vs Linux code). > > This and the previous request to split using linux commit will actually end up in 10 patches or more. I think we need to differentiate the two request. The previous request is about logging which commits you backported. I would be open to have all of them in one patch so long we account the authors/tags properly. For this request, this is mostly about avoid to mix multiple things together. Your commit message describes 3 distinct parts and therefore they should be split. >>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >>> --- >>> xen/arch/arm/arm64/cpufeature.c | 18 +++++- >>> xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- >>> xen/arch/arm/include/asm/cpufeature.h | 14 ++++- >>> 3 files changed, 91 insertions(+), 17 deletions(-) >>> diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c >>> index 6e5d30dc7b..d9039d37b2 100644 >>> --- a/xen/arch/arm/arm64/cpufeature.c >>> +++ b/xen/arch/arm/arm64/cpufeature.c >>> @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = { >>> ARM64_FTR_END, >>> }; >>> +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { >>> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), >>> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >>> + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), >>> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >> So we are using CONFIG_ARM64_PTR_AUTH. But this is not defined in Kconfig. I realize there are more in cpufeature.c (somehow I didn't spot during preview), but I don't think this is right to define CONFIG_* without an associated entry in Kconfig. >> >> In one hand, I think it would be odd to add an entry in Kconfig because Xen wouldn't properly work if selected. On the other hand, it is useful if when we will implement pointer authentification. >> >> So maybe we should just add the Kconfig entry with a comment explaning why they are not selected. Any thoughts? > > This is really right and a very good catch. > > I think it would make sense to introduce those in Kconfig in order to keep the code equivalent to Linux. > > So I would suggest here to add hidden entries like this: > > ARM64_PTR_AUTH > def_bool n > depends on ARM64 > help > Pointer authentication support. > This feature is not supported by Xen. I am OK with that. Cheers,
Hi Julien, > On 4 May 2022, at 09:20, Julien Grall <julien@xen.org> wrote: > > > > On 04/05/2022 08:39, Bertrand Marquis wrote: >> Hi Julien, > Hi, > >>> On 3 May 2022, at 19:08, Julien Grall <julien@xen.org> wrote: >>> >>> Hi Bertrand, >>> >>> On 03/05/2022 10:38, Bertrand Marquis wrote: >>>> Sync arm64 sysreg bit shift definitions with status of Linux kernel as >>>> of 5.18-rc3 version (linux commit b2d229d4ddb1). >>>> Sync ID registers sanitization with the status of Linux 5.18-rc3 and add >>>> sanitization of ISAR2 registers. >>> Please outline which specific commits you are actually backported. This would help to know what changed, why and also keep track of the autorships. >>> >>> When possible, the changes should be separated to match each Linux commit we backport. >> As those are exactly identical to the linux tree, one can easily use git blame on the linux source tree to find those information if it is needed > > Well... that's possible at the cost of everyone going through Linux to understand why the changes were made. This is not very scalable. > >> I checked a bit and this is not something that was required before (for example when the cpufeature was introduced). > > If we import the full file, then we will generally don't log all the commits. However, for smaller changes, we will always mention the commit backported. There are several examples on the ML: > > - 0435784cc75d ("xen/arm: smmuv1: Intelligent SMR allocation") > - 9c432b876bf5 ("x86/mwait-idle: add SPR support") > > We also recently introduced a tag "Origin:" to keep track of which commit was backported. If you want to understand the rationale, you can read this long thread: > > https://lore.kernel.org/xen-devel/0ed245fa-58a7-a5f6-b82e-48f9ed0b6970@suse.com/ Do I understand right that it is ok for you if I push one patch mentioning all the commits done in Linux corresponding to the changes (instead of one patch per commit) ? > >>> >>>> Complete AA64ISAR2 and AA64MMFR1 with more fields. >>>> While there add a comment for MMFR bitfields as for other registers in >>>> the cpuinfo structure definition. >>> >>> AFAICT, this patch is doing 3 different things that are somewhat related: >>> - Sync cpufeature.c >>> - Update the headers with unused defines >>> - Complete the structure cpufeature.h >>> >>> All those changes seem to be independent, so I think they should be done separately. This would help to keep the authorship right (your code vs Linux code). >> This and the previous request to split using linux commit will actually end up in 10 patches or more. > > I think we need to differentiate the two request. The previous request is about logging which commits you backported. I would be open to have all of them in one patch so long we account the authors/tags properly. > > For this request, this is mostly about avoid to mix multiple things together. Your commit message describes 3 distinct parts and therefore they should be split. So 3 patches if you confirm the previous point. I am ok with that > >>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >>>> --- >>>> xen/arch/arm/arm64/cpufeature.c | 18 +++++- >>>> xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- >>>> xen/arch/arm/include/asm/cpufeature.h | 14 ++++- >>>> 3 files changed, 91 insertions(+), 17 deletions(-) >>>> diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c >>>> index 6e5d30dc7b..d9039d37b2 100644 >>>> --- a/xen/arch/arm/arm64/cpufeature.c >>>> +++ b/xen/arch/arm/arm64/cpufeature.c >>>> @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = { >>>> ARM64_FTR_END, >>>> }; >>>> +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { >>>> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), >>>> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >>>> + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), >>>> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >>> So we are using CONFIG_ARM64_PTR_AUTH. But this is not defined in Kconfig. I realize there are more in cpufeature.c (somehow I didn't spot during preview), but I don't think this is right to define CONFIG_* without an associated entry in Kconfig. >>> >>> In one hand, I think it would be odd to add an entry in Kconfig because Xen wouldn't properly work if selected. On the other hand, it is useful if when we will implement pointer authentification. >>> >>> So maybe we should just add the Kconfig entry with a comment explaning why they are not selected. Any thoughts? >> This is really right and a very good catch. >> I think it would make sense to introduce those in Kconfig in order to keep the code equivalent to Linux. >> So I would suggest here to add hidden entries like this: >> ARM64_PTR_AUTH >> def_bool n >> depends on ARM64 >> help >> Pointer authentication support. >> This feature is not supported by Xen. > > I am OK with that. Ok, there are currently 4 CONFIG_ not defined so I will add a patch for those in my serie. Cheers Bertrand > > Cheers, > > -- > Julien Grall
Hi Bertrand, On 04/05/2022 10:49, Bertrand Marquis wrote: >> On 4 May 2022, at 09:20, Julien Grall <julien@xen.org> wrote: >> >> >> >> On 04/05/2022 08:39, Bertrand Marquis wrote: >>> Hi Julien, >> Hi, >> >>>> On 3 May 2022, at 19:08, Julien Grall <julien@xen.org> wrote: >>>> >>>> Hi Bertrand, >>>> >>>> On 03/05/2022 10:38, Bertrand Marquis wrote: >>>>> Sync arm64 sysreg bit shift definitions with status of Linux kernel as >>>>> of 5.18-rc3 version (linux commit b2d229d4ddb1). >>>>> Sync ID registers sanitization with the status of Linux 5.18-rc3 and add >>>>> sanitization of ISAR2 registers. >>>> Please outline which specific commits you are actually backported. This would help to know what changed, why and also keep track of the autorships. >>>> >>>> When possible, the changes should be separated to match each Linux commit we backport. >>> As those are exactly identical to the linux tree, one can easily use git blame on the linux source tree to find those information if it is needed >> >> Well... that's possible at the cost of everyone going through Linux to understand why the changes were made. This is not very scalable. >> >>> I checked a bit and this is not something that was required before (for example when the cpufeature was introduced). >> >> If we import the full file, then we will generally don't log all the commits. However, for smaller changes, we will always mention the commit backported. There are several examples on the ML: >> >> - 0435784cc75d ("xen/arm: smmuv1: Intelligent SMR allocation") >> - 9c432b876bf5 ("x86/mwait-idle: add SPR support") >> >> We also recently introduced a tag "Origin:" to keep track of which commit was backported. If you want to understand the rationale, you can read this long thread: >> >> https://lore.kernel.org/xen-devel/0ed245fa-58a7-a5f6-b82e-48f9ed0b6970@suse.com/ > > Do I understand right that it is ok for you if I push one patch mentioning all the commits done in Linux corresponding to the changes (instead of one patch per commit) ? For this case yes. Cheers,
On Wed, 4 May 2022, Julien Grall wrote: > > Do I understand right that it is ok for you if I push one patch mentioning > > all the commits done in Linux corresponding to the changes (instead of one > > patch per commit) ? > > For this case yes. I managed to do a review of the patch by doing a diff of the relevant portion of Xen cpufeature.c with Linux cpufeature.c (from commit b2d229d4ddb1), and the relevant portion of Xen sysregs.h with Linux sysregs.h (diff -E -b -u). Everything checks out. In my opinion, this patch should be split in 2 patches: the changes to cpufeature.c and sysregs.c that come from the Linux sources; and the updates to cpufeature.h that do not. If you do that you can add my reviewed-by to the first patch with the changes from Linux. The list of individual commit IDs would be nice, but thanksfully the two source files are still "diffable" so in my opinion are not required. I have a couple of comments on the changes to cpufeature.h (the ones not from Linux) which I'll reply directly to the patch.
On Tue, 3 May 2022, Bertrand Marquis wrote: > Sync arm64 sysreg bit shift definitions with status of Linux kernel as > of 5.18-rc3 version (linux commit b2d229d4ddb1). > Sync ID registers sanitization with the status of Linux 5.18-rc3 and add > sanitization of ISAR2 registers. > Complete AA64ISAR2 and AA64MMFR1 with more fields. > While there add a comment for MMFR bitfields as for other registers in > the cpuinfo structure definition. > > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> > --- > xen/arch/arm/arm64/cpufeature.c | 18 +++++- > xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- Linux cpufeature.c has a couple more structures compared to Xen. So I would add the word "existing" in the commit message: "Sync existing ID registers sanitization with the status of Linux 5.18-rc3 and add sanitization of ISAR2 registers." A couple of comments about the cpufeature.h changes below. > xen/arch/arm/include/asm/cpufeature.h | 14 ++++- > 3 files changed, 91 insertions(+), 17 deletions(-) > > diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c > index 6e5d30dc7b..d9039d37b2 100644 > --- a/xen/arch/arm/arm64/cpufeature.c > +++ b/xen/arch/arm/arm64/cpufeature.c > @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = { > ARM64_FTR_END, > }; > > +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { > + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), > + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), > + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), > + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), > + FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_GPA3_SHIFT, 4, 0), > + ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_RPRES_SHIFT, 4, 0), > + ARM64_FTR_END, > +}; > + > static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { > ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_CSV3_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_CSV2_SHIFT, 4, 0), > @@ -158,8 +168,8 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { > S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL3_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL2_SHIFT, 4, 0), > - ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_EL1_64BIT_ONLY), > - ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_EL0_64BIT_ONLY), > + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_ELx_64BIT_ONLY), > + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_ELx_64BIT_ONLY), > ARM64_FTR_END, > }; > > @@ -197,7 +207,7 @@ static const struct arm64_ftr_bits ftr_id_aa64zfr0[] = { > }; > > static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = { > - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_ECV_SHIFT, 4, 0), > + ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_ECV_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_FGT_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_EXS_SHIFT, 4, 0), > /* > @@ -243,6 +253,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = { > }; > > static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = { > + ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_AFP_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_ETS_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_TWED_SHIFT, 4, 0), > ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_XNX_SHIFT, 4, 0), > @@ -588,6 +599,7 @@ void update_system_features(const struct cpuinfo_arm *new) > > SANITIZE_ID_REG(isa64, 0, aa64isar0); > SANITIZE_ID_REG(isa64, 1, aa64isar1); > + SANITIZE_ID_REG(isa64, 2, aa64isar2); > > SANITIZE_ID_REG(zfr64, 0, aa64zfr0); > > diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h > index eac08ed33f..54670084c3 100644 > --- a/xen/arch/arm/include/asm/arm64/sysregs.h > +++ b/xen/arch/arm/include/asm/arm64/sysregs.h > @@ -144,6 +144,30 @@ > > /* id_aa64isar2 */ > #define ID_AA64ISAR2_CLEARBHB_SHIFT 28 > +#define ID_AA64ISAR2_APA3_SHIFT 12 > +#define ID_AA64ISAR2_GPA3_SHIFT 8 > +#define ID_AA64ISAR2_RPRES_SHIFT 4 > +#define ID_AA64ISAR2_WFXT_SHIFT 0 > + > +#define ID_AA64ISAR2_RPRES_8BIT 0x0 > +#define ID_AA64ISAR2_RPRES_12BIT 0x1 > +/* > + * Value 0x1 has been removed from the architecture, and is > + * reserved, but has not yet been removed from the ARM ARM > + * as of ARM DDI 0487G.b. > + */ > +#define ID_AA64ISAR2_WFXT_NI 0x0 > +#define ID_AA64ISAR2_WFXT_SUPPORTED 0x2 > + > +#define ID_AA64ISAR2_APA3_NI 0x0 > +#define ID_AA64ISAR2_APA3_ARCHITECTED 0x1 > +#define ID_AA64ISAR2_APA3_ARCH_EPAC 0x2 > +#define ID_AA64ISAR2_APA3_ARCH_EPAC2 0x3 > +#define ID_AA64ISAR2_APA3_ARCH_EPAC2_FPAC 0x4 > +#define ID_AA64ISAR2_APA3_ARCH_EPAC2_FPAC_CMB 0x5 > + > +#define ID_AA64ISAR2_GPA3_NI 0x0 > +#define ID_AA64ISAR2_GPA3_ARCHITECTED 0x1 > > /* id_aa64pfr0 */ > #define ID_AA64PFR0_CSV3_SHIFT 60 > @@ -165,14 +189,13 @@ > #define ID_AA64PFR0_AMU 0x1 > #define ID_AA64PFR0_SVE 0x1 > #define ID_AA64PFR0_RAS_V1 0x1 > +#define ID_AA64PFR0_RAS_V1P1 0x2 > #define ID_AA64PFR0_FP_NI 0xf > #define ID_AA64PFR0_FP_SUPPORTED 0x0 > #define ID_AA64PFR0_ASIMD_NI 0xf > #define ID_AA64PFR0_ASIMD_SUPPORTED 0x0 > -#define ID_AA64PFR0_EL1_64BIT_ONLY 0x1 > -#define ID_AA64PFR0_EL1_32BIT_64BIT 0x2 > -#define ID_AA64PFR0_EL0_64BIT_ONLY 0x1 > -#define ID_AA64PFR0_EL0_32BIT_64BIT 0x2 > +#define ID_AA64PFR0_ELx_64BIT_ONLY 0x1 > +#define ID_AA64PFR0_ELx_32BIT_64BIT 0x2 > > /* id_aa64pfr1 */ > #define ID_AA64PFR1_MPAMFRAC_SHIFT 16 > @@ -189,6 +212,7 @@ > #define ID_AA64PFR1_MTE_NI 0x0 > #define ID_AA64PFR1_MTE_EL0 0x1 > #define ID_AA64PFR1_MTE 0x2 > +#define ID_AA64PFR1_MTE_ASYMM 0x3 > > /* id_aa64zfr0 */ > #define ID_AA64ZFR0_F64MM_SHIFT 56 > @@ -228,17 +252,37 @@ > #define ID_AA64MMFR0_ASID_SHIFT 4 > #define ID_AA64MMFR0_PARANGE_SHIFT 0 > > -#define ID_AA64MMFR0_TGRAN4_NI 0xf > -#define ID_AA64MMFR0_TGRAN4_SUPPORTED 0x0 > -#define ID_AA64MMFR0_TGRAN64_NI 0xf > -#define ID_AA64MMFR0_TGRAN64_SUPPORTED 0x0 > -#define ID_AA64MMFR0_TGRAN16_NI 0x0 > -#define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1 > +#define ID_AA64MMFR0_ASID_8 0x0 > +#define ID_AA64MMFR0_ASID_16 0x2 > + > +#define ID_AA64MMFR0_TGRAN4_NI 0xf > +#define ID_AA64MMFR0_TGRAN4_SUPPORTED_MIN 0x0 > +#define ID_AA64MMFR0_TGRAN4_SUPPORTED_MAX 0x7 > +#define ID_AA64MMFR0_TGRAN64_NI 0xf > +#define ID_AA64MMFR0_TGRAN64_SUPPORTED_MIN 0x0 > +#define ID_AA64MMFR0_TGRAN64_SUPPORTED_MAX 0x7 > +#define ID_AA64MMFR0_TGRAN16_NI 0x0 > +#define ID_AA64MMFR0_TGRAN16_SUPPORTED_MIN 0x1 > +#define ID_AA64MMFR0_TGRAN16_SUPPORTED_MAX 0xf > + > +#define ID_AA64MMFR0_PARANGE_32 0x0 > +#define ID_AA64MMFR0_PARANGE_36 0x1 > +#define ID_AA64MMFR0_PARANGE_40 0x2 > +#define ID_AA64MMFR0_PARANGE_42 0x3 > +#define ID_AA64MMFR0_PARANGE_44 0x4 > #define ID_AA64MMFR0_PARANGE_48 0x5 > #define ID_AA64MMFR0_PARANGE_52 0x6 > > +#define ARM64_MIN_PARANGE_BITS 32 > + > +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_DEFAULT 0x0 > +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_NONE 0x1 > +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_MIN 0x2 > +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_MAX 0x7 > + > /* id_aa64mmfr1 */ > #define ID_AA64MMFR1_ECBHB_SHIFT 60 > +#define ID_AA64MMFR1_AFP_SHIFT 44 > #define ID_AA64MMFR1_ETS_SHIFT 36 > #define ID_AA64MMFR1_TWED_SHIFT 32 > #define ID_AA64MMFR1_XNX_SHIFT 28 > @@ -271,6 +315,9 @@ > #define ID_AA64MMFR2_CNP_SHIFT 0 > > /* id_aa64dfr0 */ > +#define ID_AA64DFR0_MTPMU_SHIFT 48 > +#define ID_AA64DFR0_TRBE_SHIFT 44 > +#define ID_AA64DFR0_TRACE_FILT_SHIFT 40 > #define ID_AA64DFR0_DOUBLELOCK_SHIFT 36 > #define ID_AA64DFR0_PMSVER_SHIFT 32 > #define ID_AA64DFR0_CTX_CMPS_SHIFT 28 > @@ -284,11 +331,18 @@ > #define ID_AA64DFR0_PMUVER_8_1 0x4 > #define ID_AA64DFR0_PMUVER_8_4 0x5 > #define ID_AA64DFR0_PMUVER_8_5 0x6 > +#define ID_AA64DFR0_PMUVER_8_7 0x7 > #define ID_AA64DFR0_PMUVER_IMP_DEF 0xf > > +#define ID_AA64DFR0_PMSVER_8_2 0x1 > +#define ID_AA64DFR0_PMSVER_8_3 0x2 > + > #define ID_DFR0_PERFMON_SHIFT 24 > > -#define ID_DFR0_PERFMON_8_1 0x4 > +#define ID_DFR0_PERFMON_8_0 0x3 > +#define ID_DFR0_PERFMON_8_1 0x4 > +#define ID_DFR0_PERFMON_8_4 0x5 > +#define ID_DFR0_PERFMON_8_5 0x6 > > #define ID_ISAR4_SWP_FRAC_SHIFT 28 > #define ID_ISAR4_PSR_M_SHIFT 24 > diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h > index f7368766c0..4719de47f3 100644 > --- a/xen/arch/arm/include/asm/cpufeature.h > +++ b/xen/arch/arm/include/asm/cpufeature.h > @@ -230,6 +230,7 @@ struct cpuinfo_arm { > union { > register_t bits[3]; > struct { > + /* MMFR0 */ > unsigned long pa_range:4; > unsigned long asid_bits:4; > unsigned long bigend:4; > @@ -240,16 +241,21 @@ struct cpuinfo_arm { > unsigned long tgranule_4K:4; > unsigned long __res0:32; > > + /* MMFR1 */ > unsigned long hafdbs:4; > unsigned long vmid_bits:4; > unsigned long vh:4; > unsigned long hpds:4; > unsigned long lo:4; > unsigned long pan:4; > - unsigned long __res1:8; > - unsigned long __res2:28; > + unsigned long specsei:4; > + unsigned long xnx:4; > + unsigned long twed:4; > + unsigned long ets:4; > + unsigned long __res1:20; We might as well complete the fields by also adding hcx, afp, ntlbpa, tidcp1, cmow. What do you think? > unsigned long ecbhb:4; > > + /* MMFR2 */ > unsigned long __res3:64; > }; > } mm64; > @@ -293,7 +299,9 @@ struct cpuinfo_arm { > unsigned long __res2:8; > > /* ISAR2 */ > - unsigned long __res3:28; > + unsigned long wfxt:4; > + unsigned long rpres:4; > + unsigned long __res3:20; Also here we can add gpa3, apa3, mops, bc, and pac_frac? > unsigned long clearbhb:4; > > unsigned long __res4:32;
> On 10 May 2022, at 03:03, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Wed, 4 May 2022, Julien Grall wrote: >>> Do I understand right that it is ok for you if I push one patch mentioning >>> all the commits done in Linux corresponding to the changes (instead of one >>> patch per commit) ? >> >> For this case yes. > > I managed to do a review of the patch by doing a diff of the relevant > portion of Xen cpufeature.c with Linux cpufeature.c (from commit > b2d229d4ddb1), and the relevant portion of Xen sysregs.h with Linux > sysregs.h (diff -E -b -u). > > Everything checks out. > > In my opinion, this patch should be split in 2 patches: the changes to > cpufeature.c and sysregs.c that come from the Linux sources; and the > updates to cpufeature.h that do not. If you do that you can add my > reviewed-by to the first patch with the changes from Linux. > > The list of individual commit IDs would be nice, but thanksfully the two > source files are still "diffable" so in my opinion are not required. I agree with that. Julien: Do you agree if I just put the changes to cpufeature.h in a separate patch ? I started to list the commit IDs corresponding to the changes in Linux and this would end up with 5 or more which I do not think would be that useful as the diff can be easily done as Stefano mentioned. > > I have a couple of comments on the changes to cpufeature.h (the ones not > from Linux) which I'll reply directly to the patch. I will answer to them. Cheers Bertrand
Hi Stefano, > On 10 May 2022, at 03:04, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Tue, 3 May 2022, Bertrand Marquis wrote: >> Sync arm64 sysreg bit shift definitions with status of Linux kernel as >> of 5.18-rc3 version (linux commit b2d229d4ddb1). >> Sync ID registers sanitization with the status of Linux 5.18-rc3 and add >> sanitization of ISAR2 registers. >> Complete AA64ISAR2 and AA64MMFR1 with more fields. >> While there add a comment for MMFR bitfields as for other registers in >> the cpuinfo structure definition. >> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> >> --- >> xen/arch/arm/arm64/cpufeature.c | 18 +++++- >> xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- > > Linux cpufeature.c has a couple more structures compared to Xen. So I > would add the word "existing" in the commit message: > > "Sync existing ID registers sanitization with the status of Linux > 5.18-rc3 and add sanitization of ISAR2 registers." Ok will do. > > A couple of comments about the cpufeature.h changes below. > > >> xen/arch/arm/include/asm/cpufeature.h | 14 ++++- >> 3 files changed, 91 insertions(+), 17 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c >> index 6e5d30dc7b..d9039d37b2 100644 >> --- a/xen/arch/arm/arm64/cpufeature.c >> +++ b/xen/arch/arm/arm64/cpufeature.c >> @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = { >> ARM64_FTR_END, >> }; >> >> +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >> + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >> + FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_GPA3_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_RPRES_SHIFT, 4, 0), >> + ARM64_FTR_END, >> +}; >> + >> static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_CSV3_SHIFT, 4, 0), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_CSV2_SHIFT, 4, 0), >> @@ -158,8 +168,8 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { >> S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL3_SHIFT, 4, 0), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL2_SHIFT, 4, 0), >> - ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_EL1_64BIT_ONLY), >> - ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_EL0_64BIT_ONLY), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_ELx_64BIT_ONLY), >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_ELx_64BIT_ONLY), >> ARM64_FTR_END, >> }; >> >> @@ -197,7 +207,7 @@ static const struct arm64_ftr_bits ftr_id_aa64zfr0[] = { >> }; >> >> static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = { >> - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_ECV_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_ECV_SHIFT, 4, 0), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_FGT_SHIFT, 4, 0), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_EXS_SHIFT, 4, 0), >> /* >> @@ -243,6 +253,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = { >> }; >> >> static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = { >> + ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_AFP_SHIFT, 4, 0), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_ETS_SHIFT, 4, 0), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_TWED_SHIFT, 4, 0), >> ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_XNX_SHIFT, 4, 0), >> @@ -588,6 +599,7 @@ void update_system_features(const struct cpuinfo_arm *new) >> >> SANITIZE_ID_REG(isa64, 0, aa64isar0); >> SANITIZE_ID_REG(isa64, 1, aa64isar1); >> + SANITIZE_ID_REG(isa64, 2, aa64isar2); >> >> SANITIZE_ID_REG(zfr64, 0, aa64zfr0); >> >> diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h >> index eac08ed33f..54670084c3 100644 >> --- a/xen/arch/arm/include/asm/arm64/sysregs.h >> +++ b/xen/arch/arm/include/asm/arm64/sysregs.h >> @@ -144,6 +144,30 @@ >> >> /* id_aa64isar2 */ >> #define ID_AA64ISAR2_CLEARBHB_SHIFT 28 >> +#define ID_AA64ISAR2_APA3_SHIFT 12 >> +#define ID_AA64ISAR2_GPA3_SHIFT 8 >> +#define ID_AA64ISAR2_RPRES_SHIFT 4 >> +#define ID_AA64ISAR2_WFXT_SHIFT 0 >> + >> +#define ID_AA64ISAR2_RPRES_8BIT 0x0 >> +#define ID_AA64ISAR2_RPRES_12BIT 0x1 >> +/* >> + * Value 0x1 has been removed from the architecture, and is >> + * reserved, but has not yet been removed from the ARM ARM >> + * as of ARM DDI 0487G.b. >> + */ >> +#define ID_AA64ISAR2_WFXT_NI 0x0 >> +#define ID_AA64ISAR2_WFXT_SUPPORTED 0x2 >> + >> +#define ID_AA64ISAR2_APA3_NI 0x0 >> +#define ID_AA64ISAR2_APA3_ARCHITECTED 0x1 >> +#define ID_AA64ISAR2_APA3_ARCH_EPAC 0x2 >> +#define ID_AA64ISAR2_APA3_ARCH_EPAC2 0x3 >> +#define ID_AA64ISAR2_APA3_ARCH_EPAC2_FPAC 0x4 >> +#define ID_AA64ISAR2_APA3_ARCH_EPAC2_FPAC_CMB 0x5 >> + >> +#define ID_AA64ISAR2_GPA3_NI 0x0 >> +#define ID_AA64ISAR2_GPA3_ARCHITECTED 0x1 >> >> /* id_aa64pfr0 */ >> #define ID_AA64PFR0_CSV3_SHIFT 60 >> @@ -165,14 +189,13 @@ >> #define ID_AA64PFR0_AMU 0x1 >> #define ID_AA64PFR0_SVE 0x1 >> #define ID_AA64PFR0_RAS_V1 0x1 >> +#define ID_AA64PFR0_RAS_V1P1 0x2 >> #define ID_AA64PFR0_FP_NI 0xf >> #define ID_AA64PFR0_FP_SUPPORTED 0x0 >> #define ID_AA64PFR0_ASIMD_NI 0xf >> #define ID_AA64PFR0_ASIMD_SUPPORTED 0x0 >> -#define ID_AA64PFR0_EL1_64BIT_ONLY 0x1 >> -#define ID_AA64PFR0_EL1_32BIT_64BIT 0x2 >> -#define ID_AA64PFR0_EL0_64BIT_ONLY 0x1 >> -#define ID_AA64PFR0_EL0_32BIT_64BIT 0x2 >> +#define ID_AA64PFR0_ELx_64BIT_ONLY 0x1 >> +#define ID_AA64PFR0_ELx_32BIT_64BIT 0x2 >> >> /* id_aa64pfr1 */ >> #define ID_AA64PFR1_MPAMFRAC_SHIFT 16 >> @@ -189,6 +212,7 @@ >> #define ID_AA64PFR1_MTE_NI 0x0 >> #define ID_AA64PFR1_MTE_EL0 0x1 >> #define ID_AA64PFR1_MTE 0x2 >> +#define ID_AA64PFR1_MTE_ASYMM 0x3 >> >> /* id_aa64zfr0 */ >> #define ID_AA64ZFR0_F64MM_SHIFT 56 >> @@ -228,17 +252,37 @@ >> #define ID_AA64MMFR0_ASID_SHIFT 4 >> #define ID_AA64MMFR0_PARANGE_SHIFT 0 >> >> -#define ID_AA64MMFR0_TGRAN4_NI 0xf >> -#define ID_AA64MMFR0_TGRAN4_SUPPORTED 0x0 >> -#define ID_AA64MMFR0_TGRAN64_NI 0xf >> -#define ID_AA64MMFR0_TGRAN64_SUPPORTED 0x0 >> -#define ID_AA64MMFR0_TGRAN16_NI 0x0 >> -#define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1 >> +#define ID_AA64MMFR0_ASID_8 0x0 >> +#define ID_AA64MMFR0_ASID_16 0x2 >> + >> +#define ID_AA64MMFR0_TGRAN4_NI 0xf >> +#define ID_AA64MMFR0_TGRAN4_SUPPORTED_MIN 0x0 >> +#define ID_AA64MMFR0_TGRAN4_SUPPORTED_MAX 0x7 >> +#define ID_AA64MMFR0_TGRAN64_NI 0xf >> +#define ID_AA64MMFR0_TGRAN64_SUPPORTED_MIN 0x0 >> +#define ID_AA64MMFR0_TGRAN64_SUPPORTED_MAX 0x7 >> +#define ID_AA64MMFR0_TGRAN16_NI 0x0 >> +#define ID_AA64MMFR0_TGRAN16_SUPPORTED_MIN 0x1 >> +#define ID_AA64MMFR0_TGRAN16_SUPPORTED_MAX 0xf >> + >> +#define ID_AA64MMFR0_PARANGE_32 0x0 >> +#define ID_AA64MMFR0_PARANGE_36 0x1 >> +#define ID_AA64MMFR0_PARANGE_40 0x2 >> +#define ID_AA64MMFR0_PARANGE_42 0x3 >> +#define ID_AA64MMFR0_PARANGE_44 0x4 >> #define ID_AA64MMFR0_PARANGE_48 0x5 >> #define ID_AA64MMFR0_PARANGE_52 0x6 >> >> +#define ARM64_MIN_PARANGE_BITS 32 >> + >> +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_DEFAULT 0x0 >> +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_NONE 0x1 >> +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_MIN 0x2 >> +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_MAX 0x7 >> + >> /* id_aa64mmfr1 */ >> #define ID_AA64MMFR1_ECBHB_SHIFT 60 >> +#define ID_AA64MMFR1_AFP_SHIFT 44 >> #define ID_AA64MMFR1_ETS_SHIFT 36 >> #define ID_AA64MMFR1_TWED_SHIFT 32 >> #define ID_AA64MMFR1_XNX_SHIFT 28 >> @@ -271,6 +315,9 @@ >> #define ID_AA64MMFR2_CNP_SHIFT 0 >> >> /* id_aa64dfr0 */ >> +#define ID_AA64DFR0_MTPMU_SHIFT 48 >> +#define ID_AA64DFR0_TRBE_SHIFT 44 >> +#define ID_AA64DFR0_TRACE_FILT_SHIFT 40 >> #define ID_AA64DFR0_DOUBLELOCK_SHIFT 36 >> #define ID_AA64DFR0_PMSVER_SHIFT 32 >> #define ID_AA64DFR0_CTX_CMPS_SHIFT 28 >> @@ -284,11 +331,18 @@ >> #define ID_AA64DFR0_PMUVER_8_1 0x4 >> #define ID_AA64DFR0_PMUVER_8_4 0x5 >> #define ID_AA64DFR0_PMUVER_8_5 0x6 >> +#define ID_AA64DFR0_PMUVER_8_7 0x7 >> #define ID_AA64DFR0_PMUVER_IMP_DEF 0xf >> >> +#define ID_AA64DFR0_PMSVER_8_2 0x1 >> +#define ID_AA64DFR0_PMSVER_8_3 0x2 >> + >> #define ID_DFR0_PERFMON_SHIFT 24 >> >> -#define ID_DFR0_PERFMON_8_1 0x4 >> +#define ID_DFR0_PERFMON_8_0 0x3 >> +#define ID_DFR0_PERFMON_8_1 0x4 >> +#define ID_DFR0_PERFMON_8_4 0x5 >> +#define ID_DFR0_PERFMON_8_5 0x6 >> >> #define ID_ISAR4_SWP_FRAC_SHIFT 28 >> #define ID_ISAR4_PSR_M_SHIFT 24 >> diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h >> index f7368766c0..4719de47f3 100644 >> --- a/xen/arch/arm/include/asm/cpufeature.h >> +++ b/xen/arch/arm/include/asm/cpufeature.h >> @@ -230,6 +230,7 @@ struct cpuinfo_arm { >> union { >> register_t bits[3]; >> struct { >> + /* MMFR0 */ >> unsigned long pa_range:4; >> unsigned long asid_bits:4; >> unsigned long bigend:4; >> @@ -240,16 +241,21 @@ struct cpuinfo_arm { >> unsigned long tgranule_4K:4; >> unsigned long __res0:32; >> >> + /* MMFR1 */ >> unsigned long hafdbs:4; >> unsigned long vmid_bits:4; >> unsigned long vh:4; >> unsigned long hpds:4; >> unsigned long lo:4; >> unsigned long pan:4; >> - unsigned long __res1:8; >> - unsigned long __res2:28; >> + unsigned long specsei:4; >> + unsigned long xnx:4; >> + unsigned long twed:4; >> + unsigned long ets:4; >> + unsigned long __res1:20; > > We might as well complete the fields by also adding hcx, afp, ntlbpa, > tidcp1, cmow. What do you think? Ok. > > >> unsigned long ecbhb:4; >> >> + /* MMFR2 */ >> unsigned long __res3:64; >> }; >> } mm64; >> @@ -293,7 +299,9 @@ struct cpuinfo_arm { >> unsigned long __res2:8; >> >> /* ISAR2 */ >> - unsigned long __res3:28; >> + unsigned long wfxt:4; >> + unsigned long rpres:4; >> + unsigned long __res3:20; > > Also here we can add gpa3, apa3, mops, bc, and pac_frac? Ok. Cheers Bertrand > > >> unsigned long clearbhb:4; >> >> unsigned long __res4:32;
Hi, On 11/05/2022 15:41, Bertrand Marquis wrote: > > >> On 10 May 2022, at 03:03, Stefano Stabellini <sstabellini@kernel.org> wrote: >> >> On Wed, 4 May 2022, Julien Grall wrote: >>>> Do I understand right that it is ok for you if I push one patch mentioning >>>> all the commits done in Linux corresponding to the changes (instead of one >>>> patch per commit) ? >>> >>> For this case yes. >> >> I managed to do a review of the patch by doing a diff of the relevant >> portion of Xen cpufeature.c with Linux cpufeature.c (from commit >> b2d229d4ddb1), and the relevant portion of Xen sysregs.h with Linux >> sysregs.h (diff -E -b -u). >> >> Everything checks out. >> >> In my opinion, this patch should be split in 2 patches: the changes to >> cpufeature.c and sysregs.c that come from the Linux sources; and the >> updates to cpufeature.h that do not. If you do that you can add my >> reviewed-by to the first patch with the changes from Linux. >> >> The list of individual commit IDs would be nice, but thanksfully the two >> source files are still "diffable" so in my opinion are not required. > > I agree with that. > > Julien: Do you agree if I just put the changes to cpufeature.h in a separate patch ? > > I started to list the commit IDs corresponding to the changes in Linux and this would > end up with 5 or more which I do not think would be that useful as the diff can be easily > done as Stefano mentioned. It looks like there are some confusion why I asked the list of commit. For this case, this is not about diffing the code (it is easy to do and I have already done that). It is more about authorship and where the patches come from. Technically, speaking you only copied the code from Linux and therefore you are not the author of some of the changes. For such case, our general process is: 1) Backport the commit as-is (i.e the Author is the original Author) 2) Add the tag Origin (recently introduced) 3) Add your signed-off-by I understand the patch is already written, so I was OK if you simply list of the commits with the authors/tags for this time. If both Stefano and you agree to not keep the authorships, then I will not stand against it. However, I will not get involved in committing and adding my ack. Cheers,
Hi Julien, > On 11 May 2022, at 16:20, Julien Grall <julien@xen.org> wrote: > > Hi, > > On 11/05/2022 15:41, Bertrand Marquis wrote: >>> On 10 May 2022, at 03:03, Stefano Stabellini <sstabellini@kernel.org> wrote: >>> >>> On Wed, 4 May 2022, Julien Grall wrote: >>>>> Do I understand right that it is ok for you if I push one patch mentioning >>>>> all the commits done in Linux corresponding to the changes (instead of one >>>>> patch per commit) ? >>>> >>>> For this case yes. >>> >>> I managed to do a review of the patch by doing a diff of the relevant >>> portion of Xen cpufeature.c with Linux cpufeature.c (from commit >>> b2d229d4ddb1), and the relevant portion of Xen sysregs.h with Linux >>> sysregs.h (diff -E -b -u). >>> >>> Everything checks out. >>> >>> In my opinion, this patch should be split in 2 patches: the changes to >>> cpufeature.c and sysregs.c that come from the Linux sources; and the >>> updates to cpufeature.h that do not. If you do that you can add my >>> reviewed-by to the first patch with the changes from Linux. >>> >>> The list of individual commit IDs would be nice, but thanksfully the two >>> source files are still "diffable" so in my opinion are not required. >> I agree with that. >> Julien: Do you agree if I just put the changes to cpufeature.h in a separate patch ? >> I started to list the commit IDs corresponding to the changes in Linux and this would >> end up with 5 or more which I do not think would be that useful as the diff can be easily >> done as Stefano mentioned. > > It looks like there are some confusion why I asked the list of commit. For this case, this is not about diffing the code (it is easy to do and I have already done that). It is more about authorship and where the patches come from. This is clear from the commit message as I give the commit in Linux used so the history can be easily found from that. I am a bit lost on the authorship part ... > > Technically, speaking you only copied the code from Linux and therefore you are not the author of some of the changes. > > For such case, our general process is: Could you tell me where this process is described ? > 1) Backport the commit as-is (i.e the Author is the original Author) > 2) Add the tag Origin (recently introduced) > 3) Add your signed-off-by So following this theory, if we import a file from Linux we should list all the people who contributed to it since it was created ? > > I understand the patch is already written, so I was OK if you simply list of the commits with the authors/tags for this time. I would like to understand where this requirement is coming from. @George: is there some kind of legal reason for something like that ? > > If both Stefano and you agree to not keep the authorships, then I will not stand against it. However, I will not get involved in committing and adding my ack. I want first to clear up this process and understand why you are requesting this to know how I should do anything like that in the future. Cheers Bertrand > > Cheers, > > -- > Julien Grall
Hi Bertrand, On 11/05/2022 16:40, Bertrand Marquis wrote: >> On 11 May 2022, at 16:20, Julien Grall <julien@xen.org> wrote: >> Technically, speaking you only copied the code from Linux and therefore you are not the author of some of the changes. >> >> For such case, our general process is: > > Could you tell me where this process is described ? The closest description I could find is: https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/process/sending-patches.pandoc;h=7ff7826c992b68804b41cc4a8605329f7a10e44c;hb=HEAD#l75 > >> 1) Backport the commit as-is (i.e the Author is the original Author) >> 2) Add the tag Origin (recently introduced) >> 3) Add your signed-off-by > > So following this theory, if we import a file from Linux we should list all the people who contributed to it since it was created ? Technically yes. Cheers,
Hi, > On 11 May 2022, at 16:47, Julien Grall <julien@xen.org> wrote: > > Hi Bertrand, > > On 11/05/2022 16:40, Bertrand Marquis wrote: >>> On 11 May 2022, at 16:20, Julien Grall <julien@xen.org> wrote: >>> Technically, speaking you only copied the code from Linux and therefore you are not the author of some of the changes. >>> >>> For such case, our general process is: >> Could you tell me where this process is described ? > > The closest description I could find is: > > https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/process/sending-patches.pandoc;h=7ff7826c992b68804b41cc4a8605329f7a10e44c;hb=HEAD#l75 This state to give the origin which I can easily do for my patch. But it does not state that we need to list all the authors from all changes in the origin. Are you only requesting to modify my patch to add one origin line with the SHA or to actually list all authors of all patches for all changes between the previous state in Xen and the state after my patch ? > >>> 1) Backport the commit as-is (i.e the Author is the original Author) >>> 2) Add the tag Origin (recently introduced) >>> 3) Add your signed-off-by >> So following this theory, if we import a file from Linux we should list all the people who contributed to it since it was created ? > > Technically yes. But this does not apply here as I am not back porting a patch but syncing a file. The origin as said upper should be enough to get this information. Cheers Bertrand > > Cheers, > > -- > Julien Grall
On 11/05/2022 17:01, Bertrand Marquis wrote: > Hi, > >> On 11 May 2022, at 16:47, Julien Grall <julien@xen.org> wrote: >> >> Hi Bertrand, >> >> On 11/05/2022 16:40, Bertrand Marquis wrote: >>>> On 11 May 2022, at 16:20, Julien Grall <julien@xen.org> wrote: >>>> Technically, speaking you only copied the code from Linux and therefore you are not the author of some of the changes. >>>> >>>> For such case, our general process is: >>> Could you tell me where this process is described ? >> >> The closest description I could find is: >> >> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/process/sending-patches.pandoc;h=7ff7826c992b68804b41cc4a8605329f7a10e44c;hb=HEAD#l75 > > This state to give the origin which I can easily do for my patch. > But it does not state that we need to list all the authors from all changes in the origin. Hmmmm.... I guess we are reading the following statement differently: All tags **above** the `Origin:` tag are from the original patch (which should all be kept), while tags **after** `Origin:` are related to the normal Xen patch process as described here. > > Are you only requesting to modify my patch to add one origin line with the SHA or to actually list all authors of all patches for all changes between the previous state in Xen and the state after my patch ? > >> >>>> 1) Backport the commit as-is (i.e the Author is the original Author) >>>> 2) Add the tag Origin (recently introduced) >>>> 3) Add your signed-off-by >>> So following this theory, if we import a file from Linux we should list all the people who contributed to it since it was created ? >> >> Technically yes. > > But this does not apply here as I am not back porting a patch but syncing a file. I am not sure why you want to differentiate the two. In the latter it is actually easy to find the authors (you are syncing after all!): git log lastbaseline...newbaseline myfile.c The result could be pasted in the commit message. As the code is meant to follow Linux, then all the commits in the list should be valid for Xen. Cheers,
On Wed, 11 May 2022, Bertrand Marquis wrote: > > I understand the patch is already written, so I was OK if you simply list of the commits with the authors/tags for this time. > > I would like to understand where this requirement is coming from. > > @George: is there some kind of legal reason for something like that ? I am not George but I'll answer the legal question. Our "legal" document is the DCO: https://developercertificate.org/ This falls under case (b): (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or So from the legal point of view only your Signed-off-by line is required. I remember this well because I was confused about this a few years ago in another case of taking code from Linux. > > If both Stefano and you agree to not keep the authorships, then I will > > not stand against it. However, I will not get involved in > > committing and adding my ack. I am fine either way. My only request is to mention the Linux commit-id that Bertrand used as a base and Bertrand has already done that. > I want first to clear up this process and understand why you are > requesting this to know how I should do anything like that in the > future. It looks like our process docs are not very good on this point and might benefit from a clarification. I hope you are volunteering :-) Origin is defined as "it specifies the source of the patch" but it doesn't say what actually is considered a "source". I suggest to distinguish between the case where commits are ported individually from the case where code is copied over (like when we introduced SMMUv3.) If commits are copied individually, I think we probably want an Origin tag for each of them and the source is the original commit-id. If the code is copied from Linux (like the SMMUv3 case) then we probably only want to request a single Origin tag (or a new tag?) with the base Linux version (5.18-rc3) rather than the commit-id being backported. In that case the source would be the repository baseline. Cheers, Stefano
Hi Stefano, > On 11 May 2022, at 21:06, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Wed, 11 May 2022, Bertrand Marquis wrote: >>> I understand the patch is already written, so I was OK if you simply list of the commits with the authors/tags for this time. >> >> I would like to understand where this requirement is coming from. >> >> @George: is there some kind of legal reason for something like that ? > > I am not George but I'll answer the legal question. Our "legal" document > is the DCO: I agree with your analysis but I still think this is an area where we would need the confirmation from George. > > https://developercertificate.org/ > > This falls under case (b): > > (b) The contribution is based upon previous work that, to the best > of my knowledge, is covered under an appropriate open source > license and I have the right under that license to submit that > work with modifications, whether created in whole or in part > by me, under the same open source license (unless I am > permitted to submit under a different license), as indicated > in the file; or > > So from the legal point of view only your Signed-off-by line is > required. > > I remember this well because I was confused about this a few years ago > in another case of taking code from Linux. > > >>> If both Stefano and you agree to not keep the authorships, then I will >>> not stand against it. However, I will not get involved in >>> committing and adding my ack. > > I am fine either way. My only request is to mention the Linux commit-id > that Bertrand used as a base and Bertrand has already done that. Ok > > >> I want first to clear up this process and understand why you are >> requesting this to know how I should do anything like that in the >> future. > > It looks like our process docs are not very good on this point and might > benefit from a clarification. I hope you are volunteering :-) > > Origin is defined as "it specifies the source of the patch" but it > doesn't say what actually is considered a "source". > > I suggest to distinguish between the case where commits are ported > individually from the case where code is copied over (like when we > introduced SMMUv3.) If commits are copied individually, I think we > probably want an Origin tag for each of them and the source is the > original commit-id. If the code is copied from Linux (like the SMMUv3 > case) then we probably only want to request a single Origin tag (or a > new tag?) with the base Linux version (5.18-rc3) rather than the > commit-id being backported. In that case the source would be the > repository baseline. Before defining our own way to do that maybe we should check how others are handling those cases to not reinvent the wheel. Anybody has a suggestion of an other open source project we could check which could have the same kind of “needs” ? Cheers Bertrand > > Cheers, > > Stefano
diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c index 6e5d30dc7b..d9039d37b2 100644 --- a/xen/arch/arm/arm64/cpufeature.c +++ b/xen/arch/arm/arm64/cpufeature.c @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = { ARM64_FTR_END, }; +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), + FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_GPA3_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64ISAR2_RPRES_SHIFT, 4, 0), + ARM64_FTR_END, +}; + static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_CSV3_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_CSV2_SHIFT, 4, 0), @@ -158,8 +168,8 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = { S_ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR0_FP_SHIFT, 4, ID_AA64PFR0_FP_NI), ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL3_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL2_SHIFT, 4, 0), - ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_EL1_64BIT_ONLY), - ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_EL0_64BIT_ONLY), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL1_SHIFT, 4, ID_AA64PFR0_ELx_64BIT_ONLY), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64PFR0_EL0_SHIFT, 4, ID_AA64PFR0_ELx_64BIT_ONLY), ARM64_FTR_END, }; @@ -197,7 +207,7 @@ static const struct arm64_ftr_bits ftr_id_aa64zfr0[] = { }; static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = { - ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_ECV_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_ECV_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_FGT_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_EXS_SHIFT, 4, 0), /* @@ -243,6 +253,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = { }; static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = { + ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_AFP_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_ETS_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_TWED_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR1_XNX_SHIFT, 4, 0), @@ -588,6 +599,7 @@ void update_system_features(const struct cpuinfo_arm *new) SANITIZE_ID_REG(isa64, 0, aa64isar0); SANITIZE_ID_REG(isa64, 1, aa64isar1); + SANITIZE_ID_REG(isa64, 2, aa64isar2); SANITIZE_ID_REG(zfr64, 0, aa64zfr0); diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h index eac08ed33f..54670084c3 100644 --- a/xen/arch/arm/include/asm/arm64/sysregs.h +++ b/xen/arch/arm/include/asm/arm64/sysregs.h @@ -144,6 +144,30 @@ /* id_aa64isar2 */ #define ID_AA64ISAR2_CLEARBHB_SHIFT 28 +#define ID_AA64ISAR2_APA3_SHIFT 12 +#define ID_AA64ISAR2_GPA3_SHIFT 8 +#define ID_AA64ISAR2_RPRES_SHIFT 4 +#define ID_AA64ISAR2_WFXT_SHIFT 0 + +#define ID_AA64ISAR2_RPRES_8BIT 0x0 +#define ID_AA64ISAR2_RPRES_12BIT 0x1 +/* + * Value 0x1 has been removed from the architecture, and is + * reserved, but has not yet been removed from the ARM ARM + * as of ARM DDI 0487G.b. + */ +#define ID_AA64ISAR2_WFXT_NI 0x0 +#define ID_AA64ISAR2_WFXT_SUPPORTED 0x2 + +#define ID_AA64ISAR2_APA3_NI 0x0 +#define ID_AA64ISAR2_APA3_ARCHITECTED 0x1 +#define ID_AA64ISAR2_APA3_ARCH_EPAC 0x2 +#define ID_AA64ISAR2_APA3_ARCH_EPAC2 0x3 +#define ID_AA64ISAR2_APA3_ARCH_EPAC2_FPAC 0x4 +#define ID_AA64ISAR2_APA3_ARCH_EPAC2_FPAC_CMB 0x5 + +#define ID_AA64ISAR2_GPA3_NI 0x0 +#define ID_AA64ISAR2_GPA3_ARCHITECTED 0x1 /* id_aa64pfr0 */ #define ID_AA64PFR0_CSV3_SHIFT 60 @@ -165,14 +189,13 @@ #define ID_AA64PFR0_AMU 0x1 #define ID_AA64PFR0_SVE 0x1 #define ID_AA64PFR0_RAS_V1 0x1 +#define ID_AA64PFR0_RAS_V1P1 0x2 #define ID_AA64PFR0_FP_NI 0xf #define ID_AA64PFR0_FP_SUPPORTED 0x0 #define ID_AA64PFR0_ASIMD_NI 0xf #define ID_AA64PFR0_ASIMD_SUPPORTED 0x0 -#define ID_AA64PFR0_EL1_64BIT_ONLY 0x1 -#define ID_AA64PFR0_EL1_32BIT_64BIT 0x2 -#define ID_AA64PFR0_EL0_64BIT_ONLY 0x1 -#define ID_AA64PFR0_EL0_32BIT_64BIT 0x2 +#define ID_AA64PFR0_ELx_64BIT_ONLY 0x1 +#define ID_AA64PFR0_ELx_32BIT_64BIT 0x2 /* id_aa64pfr1 */ #define ID_AA64PFR1_MPAMFRAC_SHIFT 16 @@ -189,6 +212,7 @@ #define ID_AA64PFR1_MTE_NI 0x0 #define ID_AA64PFR1_MTE_EL0 0x1 #define ID_AA64PFR1_MTE 0x2 +#define ID_AA64PFR1_MTE_ASYMM 0x3 /* id_aa64zfr0 */ #define ID_AA64ZFR0_F64MM_SHIFT 56 @@ -228,17 +252,37 @@ #define ID_AA64MMFR0_ASID_SHIFT 4 #define ID_AA64MMFR0_PARANGE_SHIFT 0 -#define ID_AA64MMFR0_TGRAN4_NI 0xf -#define ID_AA64MMFR0_TGRAN4_SUPPORTED 0x0 -#define ID_AA64MMFR0_TGRAN64_NI 0xf -#define ID_AA64MMFR0_TGRAN64_SUPPORTED 0x0 -#define ID_AA64MMFR0_TGRAN16_NI 0x0 -#define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1 +#define ID_AA64MMFR0_ASID_8 0x0 +#define ID_AA64MMFR0_ASID_16 0x2 + +#define ID_AA64MMFR0_TGRAN4_NI 0xf +#define ID_AA64MMFR0_TGRAN4_SUPPORTED_MIN 0x0 +#define ID_AA64MMFR0_TGRAN4_SUPPORTED_MAX 0x7 +#define ID_AA64MMFR0_TGRAN64_NI 0xf +#define ID_AA64MMFR0_TGRAN64_SUPPORTED_MIN 0x0 +#define ID_AA64MMFR0_TGRAN64_SUPPORTED_MAX 0x7 +#define ID_AA64MMFR0_TGRAN16_NI 0x0 +#define ID_AA64MMFR0_TGRAN16_SUPPORTED_MIN 0x1 +#define ID_AA64MMFR0_TGRAN16_SUPPORTED_MAX 0xf + +#define ID_AA64MMFR0_PARANGE_32 0x0 +#define ID_AA64MMFR0_PARANGE_36 0x1 +#define ID_AA64MMFR0_PARANGE_40 0x2 +#define ID_AA64MMFR0_PARANGE_42 0x3 +#define ID_AA64MMFR0_PARANGE_44 0x4 #define ID_AA64MMFR0_PARANGE_48 0x5 #define ID_AA64MMFR0_PARANGE_52 0x6 +#define ARM64_MIN_PARANGE_BITS 32 + +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_DEFAULT 0x0 +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_NONE 0x1 +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_MIN 0x2 +#define ID_AA64MMFR0_TGRAN_2_SUPPORTED_MAX 0x7 + /* id_aa64mmfr1 */ #define ID_AA64MMFR1_ECBHB_SHIFT 60 +#define ID_AA64MMFR1_AFP_SHIFT 44 #define ID_AA64MMFR1_ETS_SHIFT 36 #define ID_AA64MMFR1_TWED_SHIFT 32 #define ID_AA64MMFR1_XNX_SHIFT 28 @@ -271,6 +315,9 @@ #define ID_AA64MMFR2_CNP_SHIFT 0 /* id_aa64dfr0 */ +#define ID_AA64DFR0_MTPMU_SHIFT 48 +#define ID_AA64DFR0_TRBE_SHIFT 44 +#define ID_AA64DFR0_TRACE_FILT_SHIFT 40 #define ID_AA64DFR0_DOUBLELOCK_SHIFT 36 #define ID_AA64DFR0_PMSVER_SHIFT 32 #define ID_AA64DFR0_CTX_CMPS_SHIFT 28 @@ -284,11 +331,18 @@ #define ID_AA64DFR0_PMUVER_8_1 0x4 #define ID_AA64DFR0_PMUVER_8_4 0x5 #define ID_AA64DFR0_PMUVER_8_5 0x6 +#define ID_AA64DFR0_PMUVER_8_7 0x7 #define ID_AA64DFR0_PMUVER_IMP_DEF 0xf +#define ID_AA64DFR0_PMSVER_8_2 0x1 +#define ID_AA64DFR0_PMSVER_8_3 0x2 + #define ID_DFR0_PERFMON_SHIFT 24 -#define ID_DFR0_PERFMON_8_1 0x4 +#define ID_DFR0_PERFMON_8_0 0x3 +#define ID_DFR0_PERFMON_8_1 0x4 +#define ID_DFR0_PERFMON_8_4 0x5 +#define ID_DFR0_PERFMON_8_5 0x6 #define ID_ISAR4_SWP_FRAC_SHIFT 28 #define ID_ISAR4_PSR_M_SHIFT 24 diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h index f7368766c0..4719de47f3 100644 --- a/xen/arch/arm/include/asm/cpufeature.h +++ b/xen/arch/arm/include/asm/cpufeature.h @@ -230,6 +230,7 @@ struct cpuinfo_arm { union { register_t bits[3]; struct { + /* MMFR0 */ unsigned long pa_range:4; unsigned long asid_bits:4; unsigned long bigend:4; @@ -240,16 +241,21 @@ struct cpuinfo_arm { unsigned long tgranule_4K:4; unsigned long __res0:32; + /* MMFR1 */ unsigned long hafdbs:4; unsigned long vmid_bits:4; unsigned long vh:4; unsigned long hpds:4; unsigned long lo:4; unsigned long pan:4; - unsigned long __res1:8; - unsigned long __res2:28; + unsigned long specsei:4; + unsigned long xnx:4; + unsigned long twed:4; + unsigned long ets:4; + unsigned long __res1:20; unsigned long ecbhb:4; + /* MMFR2 */ unsigned long __res3:64; }; } mm64; @@ -293,7 +299,9 @@ struct cpuinfo_arm { unsigned long __res2:8; /* ISAR2 */ - unsigned long __res3:28; + unsigned long wfxt:4; + unsigned long rpres:4; + unsigned long __res3:20; unsigned long clearbhb:4; unsigned long __res4:32;
Sync arm64 sysreg bit shift definitions with status of Linux kernel as of 5.18-rc3 version (linux commit b2d229d4ddb1). Sync ID registers sanitization with the status of Linux 5.18-rc3 and add sanitization of ISAR2 registers. Complete AA64ISAR2 and AA64MMFR1 with more fields. While there add a comment for MMFR bitfields as for other registers in the cpuinfo structure definition. Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com> --- xen/arch/arm/arm64/cpufeature.c | 18 +++++- xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- xen/arch/arm/include/asm/cpufeature.h | 14 ++++- 3 files changed, 91 insertions(+), 17 deletions(-)