| Message ID | 20210809152651.2297337-13-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clocksource/arm_arch_timer: Add basic ARMv8.6 support | expand |
Hi Marc, On Mon, Aug 9, 2021 at 8:48 AM Marc Zyngier <maz@kernel.org> wrote: > > Add a new capability to detect the Enhanced Counter Virtualization > feature (FEAT_EVC). > s/FEAT_EVC/FEAT_ECV/g > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kernel/cpufeature.c | 10 ++++++++++ > arch/arm64/tools/cpucaps | 1 + > 2 files changed, 11 insertions(+) > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index 0ead8bfedf20..9c2ce5408811 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -1899,6 +1899,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > .sign = FTR_UNSIGNED, > .min_field_value = 1, > }, > + { > + .desc = "Enhanced counter virtualization", > + .capability = ARM64_HAS_ECV, > + .type = ARM64_CPUCAP_SYSTEM_FEATURE, > + .matches = has_cpuid_feature, > + .sys_reg = SYS_ID_AA64MMFR0_EL1, > + .field_pos = ID_AA64MMFR0_ECV_SHIFT, > + .sign = FTR_UNSIGNED, > + .min_field_value = 1, > + }, Per one of your other patches in the series, it sounds like userspace access to the self-synchronized registers hasn't been settled yet. However, if/when available to userspace, should this cpufeature map to an ELF HWCAP? Also, w.r.t. my series I have out for ECV in KVM. All the controls used in EL2 depend on ECV=0x2. I agree that ECV=0x1 needs a cpufeature bit, but what about EL2's use case? Besides the typo: Reviewed-by: Oliver Upton <oupton@google.com> -- Thanks, Oliver > #ifdef CONFIG_ARM64_PAN > { > .desc = "Privileged Access Never", > diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps > index 49305c2e6dfd..7a7c58acd8f0 100644 > --- a/arch/arm64/tools/cpucaps > +++ b/arch/arm64/tools/cpucaps > @@ -18,6 +18,7 @@ HAS_CRC32 > HAS_DCPODP > HAS_DCPOP > HAS_E0PD > +HAS_ECV > HAS_EPAN > HAS_GENERIC_AUTH > HAS_GENERIC_AUTH_ARCH > -- > 2.30.2 >
On Mon, Aug 9, 2021 at 9:30 AM Oliver Upton <oupton@google.com> wrote: > > Hi Marc, > > On Mon, Aug 9, 2021 at 8:48 AM Marc Zyngier <maz@kernel.org> wrote: > > > > Add a new capability to detect the Enhanced Counter Virtualization > > feature (FEAT_EVC). > > > > s/FEAT_EVC/FEAT_ECV/g > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/kernel/cpufeature.c | 10 ++++++++++ > > arch/arm64/tools/cpucaps | 1 + > > 2 files changed, 11 insertions(+) > > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > index 0ead8bfedf20..9c2ce5408811 100644 > > --- a/arch/arm64/kernel/cpufeature.c > > +++ b/arch/arm64/kernel/cpufeature.c > > @@ -1899,6 +1899,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > > .sign = FTR_UNSIGNED, > > .min_field_value = 1, > > }, > > + { > > + .desc = "Enhanced counter virtualization", > > + .capability = ARM64_HAS_ECV, > > + .type = ARM64_CPUCAP_SYSTEM_FEATURE, > > + .matches = has_cpuid_feature, > > + .sys_reg = SYS_ID_AA64MMFR0_EL1, > > + .field_pos = ID_AA64MMFR0_ECV_SHIFT, > > + .sign = FTR_UNSIGNED, > > + .min_field_value = 1, > > + }, > > Per one of your other patches in the series, it sounds like userspace > access to the self-synchronized registers hasn't been settled yet. > However, if/when available to userspace, should this cpufeature map to > an ELF HWCAP? > > Also, w.r.t. my series I have out for ECV in KVM. All the controls > used in EL2 depend on ECV=0x2. I agree that ECV=0x1 needs a cpufeature > bit, but what about EL2's use case? Forgot to link the series: http://lore.kernel.org/r/20210804085819.846610-1-oupton@google.com > > Besides the typo: > > Reviewed-by: Oliver Upton <oupton@google.com> > > -- > Thanks, > Oliver > > > #ifdef CONFIG_ARM64_PAN > > { > > .desc = "Privileged Access Never", > > diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps > > index 49305c2e6dfd..7a7c58acd8f0 100644 > > --- a/arch/arm64/tools/cpucaps > > +++ b/arch/arm64/tools/cpucaps > > @@ -18,6 +18,7 @@ HAS_CRC32 > > HAS_DCPODP > > HAS_DCPOP > > HAS_E0PD > > +HAS_ECV > > HAS_EPAN > > HAS_GENERIC_AUTH > > HAS_GENERIC_AUTH_ARCH > > -- > > 2.30.2 > >
Hi Oliver, Thanks for having a look. On Mon, 09 Aug 2021 17:30:45 +0100, Oliver Upton <oupton@google.com> wrote: > > Hi Marc, > > On Mon, Aug 9, 2021 at 8:48 AM Marc Zyngier <maz@kernel.org> wrote: > > > > Add a new capability to detect the Enhanced Counter Virtualization > > feature (FEAT_EVC). > > > > s/FEAT_EVC/FEAT_ECV/g I'm the knig fo tpyoes :). > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/kernel/cpufeature.c | 10 ++++++++++ > > arch/arm64/tools/cpucaps | 1 + > > 2 files changed, 11 insertions(+) > > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > index 0ead8bfedf20..9c2ce5408811 100644 > > --- a/arch/arm64/kernel/cpufeature.c > > +++ b/arch/arm64/kernel/cpufeature.c > > @@ -1899,6 +1899,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > > .sign = FTR_UNSIGNED, > > .min_field_value = 1, > > }, > > + { > > + .desc = "Enhanced counter virtualization", > > + .capability = ARM64_HAS_ECV, > > + .type = ARM64_CPUCAP_SYSTEM_FEATURE, > > + .matches = has_cpuid_feature, > > + .sys_reg = SYS_ID_AA64MMFR0_EL1, > > + .field_pos = ID_AA64MMFR0_ECV_SHIFT, > > + .sign = FTR_UNSIGNED, > > + .min_field_value = 1, > > + }, > > Per one of your other patches in the series, it sounds like userspace > access to the self-synchronized registers hasn't been settled yet. > However, if/when available to userspace, should this cpufeature map to > an ELF HWCAP? We can't prevent the access to userspace, unless we also trap cntvct_el0 and cntfreq_el0. Which we try not to do. But you are indeed correct, we probably have a HWCAP if we decide to advertise it to userspace. > Also, w.r.t. my series I have out for ECV in KVM. All the controls > used in EL2 depend on ECV=0x2. I agree that ECV=0x1 needs a cpufeature > bit, but what about EL2's use case? My idea was to have a ARM64_HAS_ECV2 to capture the EL2 extensions with min_field_value=2. > Besides the typo: > > Reviewed-by: Oliver Upton <oupton@google.com> Thanks, M.
On Mon, Aug 9, 2021 at 11:02 AM Marc Zyngier <maz@kernel.org> wrote: > > Hi Oliver, > > Thanks for having a look. > > On Mon, 09 Aug 2021 17:30:45 +0100, > Oliver Upton <oupton@google.com> wrote: > > > > Hi Marc, > > > > On Mon, Aug 9, 2021 at 8:48 AM Marc Zyngier <maz@kernel.org> wrote: > > > > > > Add a new capability to detect the Enhanced Counter Virtualization > > > feature (FEAT_EVC). > > > > > > > s/FEAT_EVC/FEAT_ECV/g > > I'm the knig fo tpyoes :). > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > --- > > > arch/arm64/kernel/cpufeature.c | 10 ++++++++++ > > > arch/arm64/tools/cpucaps | 1 + > > > 2 files changed, 11 insertions(+) > > > > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > > index 0ead8bfedf20..9c2ce5408811 100644 > > > --- a/arch/arm64/kernel/cpufeature.c > > > +++ b/arch/arm64/kernel/cpufeature.c > > > @@ -1899,6 +1899,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > > > .sign = FTR_UNSIGNED, > > > .min_field_value = 1, > > > }, > > > + { > > > + .desc = "Enhanced counter virtualization", > > > + .capability = ARM64_HAS_ECV, > > > + .type = ARM64_CPUCAP_SYSTEM_FEATURE, > > > + .matches = has_cpuid_feature, > > > + .sys_reg = SYS_ID_AA64MMFR0_EL1, > > > + .field_pos = ID_AA64MMFR0_ECV_SHIFT, > > > + .sign = FTR_UNSIGNED, > > > + .min_field_value = 1, > > > + }, > > > > Per one of your other patches in the series, it sounds like userspace > > access to the self-synchronized registers hasn't been settled yet. > > However, if/when available to userspace, should this cpufeature map to > > an ELF HWCAP? > > We can't prevent the access to userspace, unless we also trap > cntvct_el0 and cntfreq_el0. Which we try not to do. But you are indeed > correct, we probably have a HWCAP if we decide to advertise it to > userspace. > > > Also, w.r.t. my series I have out for ECV in KVM. All the controls > > used in EL2 depend on ECV=0x2. I agree that ECV=0x1 needs a cpufeature > > bit, but what about EL2's use case? > > My idea was to have a ARM64_HAS_ECV2 to capture the EL2 extensions > with min_field_value=2. This SGTM. I imagine with your HWCAP patch you will be passing through ID_AA64MMFR0_EL1.ECV to userspace too. Dunno if we should clamp to 1 or let userspace see ECV=2 when we enumerate the second cpufeature. Definitely not worthy of a HWCAP, though. -- Thanks, Oliver > > Besides the typo: > > > > Reviewed-by: Oliver Upton <oupton@google.com> > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
Oh, one more thing, On Mon, Aug 9, 2021 at 11:21 AM Oliver Upton <oupton@google.com> wrote: > > On Mon, Aug 9, 2021 at 11:02 AM Marc Zyngier <maz@kernel.org> wrote: > > > > Hi Oliver, > > > > Thanks for having a look. > > > > On Mon, 09 Aug 2021 17:30:45 +0100, > > Oliver Upton <oupton@google.com> wrote: > > > > > > Hi Marc, > > > > > > On Mon, Aug 9, 2021 at 8:48 AM Marc Zyngier <maz@kernel.org> wrote: > > > > > > > > Add a new capability to detect the Enhanced Counter Virtualization > > > > feature (FEAT_EVC). > > > > > > > > > > s/FEAT_EVC/FEAT_ECV/g > > > > I'm the knig fo tpyoes :). > > > > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > > --- > > > > arch/arm64/kernel/cpufeature.c | 10 ++++++++++ > > > > arch/arm64/tools/cpucaps | 1 + > > > > 2 files changed, 11 insertions(+) > > > > > > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > > > index 0ead8bfedf20..9c2ce5408811 100644 > > > > --- a/arch/arm64/kernel/cpufeature.c > > > > +++ b/arch/arm64/kernel/cpufeature.c > > > > @@ -1899,6 +1899,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { > > > > .sign = FTR_UNSIGNED, > > > > .min_field_value = 1, > > > > }, > > > > + { > > > > + .desc = "Enhanced counter virtualization", Pesky nit: "Enhanced Counter Virtualization" > > > > + .capability = ARM64_HAS_ECV, > > > > + .type = ARM64_CPUCAP_SYSTEM_FEATURE, > > > > + .matches = has_cpuid_feature, > > > > + .sys_reg = SYS_ID_AA64MMFR0_EL1, > > > > + .field_pos = ID_AA64MMFR0_ECV_SHIFT, > > > > + .sign = FTR_UNSIGNED, > > > > + .min_field_value = 1, > > > > + }, > > > > > > Per one of your other patches in the series, it sounds like userspace > > > access to the self-synchronized registers hasn't been settled yet. > > > However, if/when available to userspace, should this cpufeature map to > > > an ELF HWCAP? > > > > We can't prevent the access to userspace, unless we also trap > > cntvct_el0 and cntfreq_el0. Which we try not to do. But you are indeed > > correct, we probably have a HWCAP if we decide to advertise it to > > userspace. > > > > > Also, w.r.t. my series I have out for ECV in KVM. All the controls > > > used in EL2 depend on ECV=0x2. I agree that ECV=0x1 needs a cpufeature > > > bit, but what about EL2's use case? > > > > My idea was to have a ARM64_HAS_ECV2 to capture the EL2 extensions > > with min_field_value=2. > > This SGTM. I imagine with your HWCAP patch you will be passing through > ID_AA64MMFR0_EL1.ECV to userspace too. Dunno if we should clamp to 1 > or let userspace see ECV=2 when we enumerate the second cpufeature. > Definitely not worthy of a HWCAP, though. > > -- > Thanks, > Oliver > > > > > Besides the typo: > > > > > > Reviewed-by: Oliver Upton <oupton@google.com> > > > > Thanks, > > > > M. > > > > -- > > Without deviation from the norm, progress is not possible.
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 0ead8bfedf20..9c2ce5408811 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -1899,6 +1899,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .sign = FTR_UNSIGNED, .min_field_value = 1, }, + { + .desc = "Enhanced counter virtualization", + .capability = ARM64_HAS_ECV, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_cpuid_feature, + .sys_reg = SYS_ID_AA64MMFR0_EL1, + .field_pos = ID_AA64MMFR0_ECV_SHIFT, + .sign = FTR_UNSIGNED, + .min_field_value = 1, + }, #ifdef CONFIG_ARM64_PAN { .desc = "Privileged Access Never", diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps index 49305c2e6dfd..7a7c58acd8f0 100644 --- a/arch/arm64/tools/cpucaps +++ b/arch/arm64/tools/cpucaps @@ -18,6 +18,7 @@ HAS_CRC32 HAS_DCPODP HAS_DCPOP HAS_E0PD +HAS_ECV HAS_EPAN HAS_GENERIC_AUTH HAS_GENERIC_AUTH_ARCH
Add a new capability to detect the Enhanced Counter Virtualization feature (FEAT_EVC). Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kernel/cpufeature.c | 10 ++++++++++ arch/arm64/tools/cpucaps | 1 + 2 files changed, 11 insertions(+)