Hi Oliver,
On Sun, Mar 27, 2022 at 3:57 PM Oliver Upton <oupton@xxxxxxxxxx> wrote:
>
> On Fri, Mar 25, 2022 at 07:35:39PM -0700, Reiji Watanabe wrote:
> > Hi Oliver,
> >
> > > > > > + */
> > > > > > +#define KVM_ARM_ID_REG_MAX_NUM 64
> > > > > > +#define IDREG_IDX(id) ((sys_reg_CRm(id) << 3) | sys_reg_Op2(id))
> > > > > > +
> > > > > > struct kvm_arch {
> > > > > > struct kvm_s2_mmu mmu;
> > > > > > @@ -137,6 +144,9 @@ struct kvm_arch {
> > > > > > /* Memory Tagging Extension enabled for the guest */
> > > > > > bool mte_enabled;
> > > > > > bool ran_once;
> > > > > > +
> > > > > > + /* ID registers for the guest. */
> > > > > > + u64 id_regs[KVM_ARM_ID_REG_MAX_NUM];
> > > > >
> > > > > This is a decently large array. Should we embed it in kvm_arch or
> > > > > allocate at init?
> > > >
> > > >
> > > > What is the reason why you think you might want to allocate it at init ?
> > >
> > > Well, its a 512 byte array of mostly cold data. We're probably
> > > convinced that the guest is going to access these registers at most once
> > > per vCPU at boot.
> > >
> > > For the vCPU context at least, we only allocate space for registers we
> > > actually care about (enum vcpu_sysreg). My impression of the feature
> > > register ranges is that there are a lot of registers which are RAZ, so I
> > > don't believe we need to make room for uninteresting values.
> > >
> > > Additionally, struct kvm is visible to EL2 if running nVHE. I
> > > don't believe hyp will ever need to look at these register values.
> >
> > As saving/restoring breakpoint/watchpoint registers for guests
> > might need a special handling when AA64DFR0_EL1.BRPs get changed,
> > next version of the series will use the data in the array at
> > EL2 nVHE. They are cold data, and almost half of the entries
> > will be RAZ at the moment though:)
>
> Shouldn't we always be doing a full context switch based on what
> registers are present in hardware? We probably don't want to leave host
> watchpoints/breakpoints visible to the guest.
Correct, any host breakpoints/watchpoints won't be exposed to the guest.
(When restoring breakpoints/watchpoints for the guest, physical
breakpoints that are not mapped to any virtual ones will be cleared)
> Additionally, if we are narrowing the guest's view of the debug
> hardware, are we going to need to start trapping debug register
> accesses? Unexposed breakpoint registers are supposed to UNDEF if we
> obey the Arm ARM to the letter. Even if we decide to not care about
> unexposed regs, I believe certain combinations of ID_AA64DF0_EL1.{BRPs,
> CTX_CMPs} might require that we emulate.
Exactly, we will need to start trapping debug registers when the special
handling is needed, and yes, we will need a special handling when the
guest accesses to invalid virtual breakpoints/watchpoints or use the
invalid virtual breakpoints as linked breakpoints.
Thanks,
Reiji
