Re: [Xen-devel] [PATCH v2 0/4] x86/HVM: implement memory read caching

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.10.18 at 15:55, <andrew.cooper3@xxxxxxxxxx> wrote:

While I haven't heard back on my earlier reply, nevertheless a few
more thoughts.

> First and foremost, I'd like recommend against trying to combine the fix
> for repeated PDPTR reading, and repeated PTE reading.  While they are
> both repeated reading problems, one really is a knoblly corner case of
> 32bit PAE paging, and one is a general emulation problem.  Fixing these
> problems independently makes the result rather more simple, and far
> closer to how real CPUs work.

That's an option, but the approach currently chosen seems to fit
better with how guest_walk_tables() works.

> Next, there should be a single hvmemul_read_once(gaddr, bytes, ...)
> (name subject to improvement), which does a transparent read-through of
> the "access once cache" in terms of a single flag guest physical address
> space.  This allows individual callers to opt into using the access-once
> semantics, and doesn't hoist them with the substantial boilerplate of
> the sole correct way to use this interface.  Furthermore, this behaviour
> has the same semantics as the correct longer term fix.

Except that guest_walk_tables() doesn't invoke any hvmemul_*()
routines, nor does it get passed a struct x86_emulate_ops. And
it shouldn't, or else it couldn't be used for shadowed PV guests
anymore. If anything we'd have to replace all guest memory
reads in guest_walk_tables() by using a caller provided function
(and writes similarly of course).

A further problem with the suggested approach are the A/D bit
updates: A generic read-once model would, as explained before,
require each logically separate read to have its own entry. Since
replay, when all memory accesses produce identical results to the
initial run, will produce the exact same access pattern, simply
maintaining a counter to index into the array (reset every time a
replay round starts) would do. The A/D bit updates, however,
need to update their respective slots, and hence need to have
a way to identify which slot it is. That's not going to be
transparent, no matter what you do, as the read function
would need to return a token to be passed to the respective
write one.

The positive side effect of going this route would be that it would
get us at least closer towards allowing guest page tables to live
in MMIO space (because an explicit dependency on being able to
map page tables would go away).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel