On Wed, Nov 11, 2020 at 01:29:29PM +0000, Marc Zyngier wrote:

On 2020-11-11 13:03, David Brazdil wrote:

quoted

quoted

quoted

+/*
+ * nVHE copy of data structures tracking available CPU cores.
+ * Only entries for CPUs that were online at KVM init are populated.
+ * Other CPUs should not be allowed to boot because their features were
+ * not checked against the finalized system capabilities.
+ */
+u64 __ro_after_init __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1]
= INVALID_HWID };

I'm not sure what __ro_after_init means once we get S2 isolation.

It is stretching the definition of 'init' a bit, I know, but I don't see
what
your worry is about S2? The intention is to mark this read-only for
.hyp.text
at runtime. With S2, the host won't be able to write to it after KVM
init.
Obviously that's currently not the case.

More importantly, EL2 can write to it at any time, which is the bit I'm
worried
about, as it makes the annotation misleading.

EL2 can't, at least not accidentally. The hyp memory mapping is PAGE_HYP_RO
(see patch 05). Does this annotation have stronger guarantees in EL1?
AFAICT, those variables are made PAGE_KERNEL_RO in mark_rodata_ro().

quoted

One thing we might change in the future is marking it RW for some
initial
setup in a HVC handler, then marking it RO for the rest of uptime.

That'd be a desirable outcome, and it would be consistent with the rest
of the kernel.

quoted

quoted

quoted

+
+u64 cpu_logical_map(int cpu)

nit: is there any reason why "cpu" cannot be unsigned? The thought
of a negative CPU number makes me shiver...

Same here. That's how it's defined in kernel proper, so I went with
that.

I'm happy to deviate from the kernel (give the function a different name
if this clashes with existing include files).

We can also fix the rest of the kernel (I've just written the trivial
patch).

Shouldn't clash with include files. Where fixing the kernel might clash is
all the users of for_each_*_cpu that use an int for the iterator var.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel