On Mon, Oct 19, 2020 at 12:43:33PM +0100, Peter Maydell wrote:
> On Mon, 19 Oct 2020 at 12:32, Dave Martin <Dave.Martin@xxxxxxx> wrote:
> > I'm not quite sure about Peter's assessment here.
> >
> > I agree with the inconsistency identified here: we always enumerate all
> > unallocated ID regs, but we enumerate ID_AA64ZFR0_EL1 conditionally.
> > This doesn't feel right: on a non-SVE guest, ID_AA64ZFR0_EL1 should
> > behave exactly as an unallocated ID register.
> >
> > I'm not sure about the proposed fix.
> >
> > For one thing, I'm not sure that old hosts will accept writing of 0 to
> > arbitrary ID regs. This may require some digging, but commit
> > 93390c0a1b20 ("arm64: KVM: Hide unsupported AArch64 CPU features from guests")
> > may be the place to start.
>
> Well, ID regs are special in the architecture -- they always exist
> and must RAZ/WI, even if they're not actually given any fields yet.
> This is different from other "unused" parts of the system register
> encoding space, which UNDEF.
Table D12-2 confirms the register should be RAZ, as it says the register
is "RO, but RAZ if SVE is not implemented". Does "RO" imply "WI", though?
For the guest we inject an exception on writes, and for userspace we
require the value to be preserved on write.
>
> Older hosts didn't permit writing 0 to all parts of the ID
> register space (and didn't list all ID registers in the KVM_GET_REG_LIST
> list), but that was a kernel bug which we've since fixed.
> (QEMU has workaround code for pre-4.15 kernels for this.)
> Across that older bugfix, migration works from an old kernel to
> a newer one, but wouldn't have worked from a post-bugfix kernel
> to a pre-4.15 one.
>
> > My original idea was that at the source end we should be conservative:
> > enumerate and dump the minimum set of registers relevant to the
> > target -- for compatibility with old hosts that don't handle the
> > unallocated ID regs at all. At the destination end, modern hosts
> > should be permissive, i.e., allow any ID reg to be set to 0, but don't
> > require the setting of any reg that older source hosts might not send.
>
> The problem is that you've actually removed registers from
> the list that were previously in it (because pre-SVE
> kernels put this ID register in the list as a RAZ/WI register,
> and now it's not in the list if SVE isn't supported).
>
> > So, I think that instead of changing the ID_AA64ZFR0_EL1 behaviour,
> > parhaps we should move all ID_UNALLOCATED() regs (and possibly
> > ID_HIDDEN(), not sure about that) to have REG_HIDDEN_USER visibility.
>
> What does this do as far as the user-facing list-of-registers
> is concerned? All these registers need to remain in the
> KVM_GET_REG_LIST list, or you break migration from an old
> kernel to a new one.
I think we should follow the spec, even for userspace access, and be RAZ
for when the feature isn't implemented. As for writes, assuming the
exception injection is what we want for the guest (not WI), then that's
correct. For userspace, I think we should continue forcing preservation
(which will force preservation of zero when it's RAZ).
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
