| Message ID | 20181217150205.27981-3-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | arm64: kvm: cache ID register trapping | expand |
On Mon, Dec 17, 2018 at 04:02:05PM +0100, Ard Biesheuvel wrote: > On SMP ARM systems, cache maintenance by set/way should only ever be > done in the context of onlining or offlining CPUs, which is typically > done by bare metal firmware and never in a virtual machine. For this > reason, we trap set/way cache maintenance operations and replace them > with conditional flushing of the entire guest address space. > > Due to this trapping, the set/way arguments passed into the set/way > ops are completely ignored, and thus irrelevant. This also means that > the set/way geometry is equally irrelevant, and we can simply report > it as 1 set and 1 way, so that legacy 32-bit ARM system software (i.e., > the kind that only receives odd fixes) doesn't take a performance hit > due to the trapping when iterating over the cachelines. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > arch/arm64/kvm/sys_regs.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 464e794b5bc5..eb244ff98dca 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1180,6 +1180,21 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > > csselr = vcpu_read_sys_reg(vcpu, CSSELR_EL1); > p->regval = get_ccsidr(csselr); > + > + /* > + * Guests should not be doing cache operations by set/way at all, and > + * for this reason, we trap them and attempt to infer the intent, so > + * that we can flush the entire guest's address space at the appropriate > + * time. > + * To prevent this trapping from causing performance problems, let's > + * expose the geometry of all data and unified caches (which are > + * guaranteed to be PIPT and thus non-aliasing) as 1 set and 1 way. > + * [If guests should attempt to infer aliasing properties from the > + * geometry (which is not permitted by the architecture), they would > + * only do so for virtually indexed caches.] > + */ > + if (!(csselr & 1)) // data or unified cache > + p->regval &= ~GENMASK(27, 2); Why are you clearing the upper bit the LineSize field? Thanks, Christoffer
On Tue, 8 Jan 2019 at 12:02, Christoffer Dall <christoffer.dall@arm.com> wrote: > > On Mon, Dec 17, 2018 at 04:02:05PM +0100, Ard Biesheuvel wrote: > > On SMP ARM systems, cache maintenance by set/way should only ever be > > done in the context of onlining or offlining CPUs, which is typically > > done by bare metal firmware and never in a virtual machine. For this > > reason, we trap set/way cache maintenance operations and replace them > > with conditional flushing of the entire guest address space. > > > > Due to this trapping, the set/way arguments passed into the set/way > > ops are completely ignored, and thus irrelevant. This also means that > > the set/way geometry is equally irrelevant, and we can simply report > > it as 1 set and 1 way, so that legacy 32-bit ARM system software (i.e., > > the kind that only receives odd fixes) doesn't take a performance hit > > due to the trapping when iterating over the cachelines. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > arch/arm64/kvm/sys_regs.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 464e794b5bc5..eb244ff98dca 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -1180,6 +1180,21 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > > > > csselr = vcpu_read_sys_reg(vcpu, CSSELR_EL1); > > p->regval = get_ccsidr(csselr); > > + > > + /* > > + * Guests should not be doing cache operations by set/way at all, and > > + * for this reason, we trap them and attempt to infer the intent, so > > + * that we can flush the entire guest's address space at the appropriate > > + * time. > > + * To prevent this trapping from causing performance problems, let's > > + * expose the geometry of all data and unified caches (which are > > + * guaranteed to be PIPT and thus non-aliasing) as 1 set and 1 way. > > + * [If guests should attempt to infer aliasing properties from the > > + * geometry (which is not permitted by the architecture), they would > > + * only do so for virtually indexed caches.] > > + */ > > + if (!(csselr & 1)) // data or unified cache > > + p->regval &= ~GENMASK(27, 2); > > Why are you clearing the upper bit the LineSize field? > Ah, that needs to be ~GENMASK(27,3), of course.
On Tue, Jan 08, 2019 at 12:11:33PM +0100, Ard Biesheuvel wrote: > On Tue, 8 Jan 2019 at 12:02, Christoffer Dall <christoffer.dall@arm.com> wrote: > > > > On Mon, Dec 17, 2018 at 04:02:05PM +0100, Ard Biesheuvel wrote: > > > On SMP ARM systems, cache maintenance by set/way should only ever be > > > done in the context of onlining or offlining CPUs, which is typically > > > done by bare metal firmware and never in a virtual machine. For this > > > reason, we trap set/way cache maintenance operations and replace them > > > with conditional flushing of the entire guest address space. > > > > > > Due to this trapping, the set/way arguments passed into the set/way > > > ops are completely ignored, and thus irrelevant. This also means that > > > the set/way geometry is equally irrelevant, and we can simply report > > > it as 1 set and 1 way, so that legacy 32-bit ARM system software (i.e., > > > the kind that only receives odd fixes) doesn't take a performance hit > > > due to the trapping when iterating over the cachelines. > > > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > --- > > > arch/arm64/kvm/sys_regs.c | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > > index 464e794b5bc5..eb244ff98dca 100644 > > > --- a/arch/arm64/kvm/sys_regs.c > > > +++ b/arch/arm64/kvm/sys_regs.c > > > @@ -1180,6 +1180,21 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > > > > > > csselr = vcpu_read_sys_reg(vcpu, CSSELR_EL1); > > > p->regval = get_ccsidr(csselr); > > > + > > > + /* > > > + * Guests should not be doing cache operations by set/way at all, and > > > + * for this reason, we trap them and attempt to infer the intent, so > > > + * that we can flush the entire guest's address space at the appropriate > > > + * time. > > > + * To prevent this trapping from causing performance problems, let's > > > + * expose the geometry of all data and unified caches (which are > > > + * guaranteed to be PIPT and thus non-aliasing) as 1 set and 1 way. > > > + * [If guests should attempt to infer aliasing properties from the > > > + * geometry (which is not permitted by the architecture), they would > > > + * only do so for virtually indexed caches.] > > > + */ > > > + if (!(csselr & 1)) // data or unified cache > > > + p->regval &= ~GENMASK(27, 2); > > > > Why are you clearing the upper bit the LineSize field? > > > > Ah, that needs to be ~GENMASK(27,3), of course. With that fixed, the patches look fine to me. Acked-by: Christoffer Dall <christoffer.dall@arm.com>
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 464e794b5bc5..eb244ff98dca 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1180,6 +1180,21 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, csselr = vcpu_read_sys_reg(vcpu, CSSELR_EL1); p->regval = get_ccsidr(csselr); + + /* + * Guests should not be doing cache operations by set/way at all, and + * for this reason, we trap them and attempt to infer the intent, so + * that we can flush the entire guest's address space at the appropriate + * time. + * To prevent this trapping from causing performance problems, let's + * expose the geometry of all data and unified caches (which are + * guaranteed to be PIPT and thus non-aliasing) as 1 set and 1 way. + * [If guests should attempt to infer aliasing properties from the + * geometry (which is not permitted by the architecture), they would + * only do so for virtually indexed caches.] + */ + if (!(csselr & 1)) // data or unified cache + p->regval &= ~GENMASK(27, 2); return true; }
On SMP ARM systems, cache maintenance by set/way should only ever be done in the context of onlining or offlining CPUs, which is typically done by bare metal firmware and never in a virtual machine. For this reason, we trap set/way cache maintenance operations and replace them with conditional flushing of the entire guest address space. Due to this trapping, the set/way arguments passed into the set/way ops are completely ignored, and thus irrelevant. This also means that the set/way geometry is equally irrelevant, and we can simply report it as 1 set and 1 way, so that legacy 32-bit ARM system software (i.e., the kind that only receives odd fixes) doesn't take a performance hit due to the trapping when iterating over the cachelines. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm64/kvm/sys_regs.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)