On 12/16/2015 04:04 AM, Jan Beulich wrote:
On 15.12.15 at 22:02,<boris.ostrov...@oracle.com> wrote:
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1778,6 +1778,28 @@ int hvm_emulate_one_no_write(
return _hvm_emulate_one(hvmemul_ctxt, &hvm_emulate_ops_no_write);
}
+static const struct x86_emulate_ops hvm_emulate_ops_mmio = {
+ .read = mmio_ro_emulated_read,
+ .insn_fetch = hvmemul_insn_fetch,
+ .write = mmio_intercept_write,
+};
Blank line missing here, but as a probably better alternative this could
move into the function below.
+int hvm_emulate_one_mmio(unsigned seg, unsigned bdf, unsigned long gla)
Name and function parameters don't match up (also for e.g. the
changed struct mmio_ro_emulate_ctxt). DYM e.g.
hvm_emulate_one_mmcfg()?
Yes. I in fact first named it *_mmcfg() and then renamed it to _*mmio().
But then by the same logic wouldn't we want to rename
mmio_intercept_write() as well, not necessarily because of arguments but
because of what it actually does?
I am not sure what you mean by your statement in parentheses about
mmio_ro_emulate_ctxt.
(The function naming in x86/mm.c
is actually using mmio because its serving wider purpose than
just MMCFG write interception. Which makes me wonder whether
we don't actually need that other part for PVH too, in which case
the naming would again be fine.)
I don't think I understand what the other part is used for in PV so I
don't know whether it is useful for PVH.
+{
+ struct mmio_ro_emulate_ctxt mmio_ro_ctxt = {
+ .cr2 = gla,
+ .seg = seg,
+ .bdf = bdf
+ };
+ struct hvm_emulate_ctxt ctxt = { .ctxt.data = &mmio_ro_ctxt };
hvm_mem_access_emulate_one() so far is the only example pre-
initializing ctxt before calling hvm_emulate_prepare(), and I don't
think it actually needs this. I think the proper place to set .data is
after the call to hvm_emulate_prepare().
I used hvm_emulate_prepare() as a convenient way to initialize the
context to a reasonable state, with definition of "reasonable" possibly
changing at some point later. Other emulating routines have
hvm_emulate_ctxt passed in so it may already have certain fields set.
(Yes, .data needs to be set afterwards).
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
