[Xen-devel] [PATCH v4 12/31] x86/mm: split out readonly MMIO emulation code

2017-08-17 Thread Wei Liu
Move the code to pv/emul-mmio-op.c. Fix coding style issues while
moving.

Note that mmio_ro_emulated_write is needed by both PV and HVM, so it
is left in x86/mm.c.

Signed-off-by: Wei Liu 
---
 xen/arch/x86/mm.c  | 129 
 xen/arch/x86/pv/Makefile   |   1 +
 xen/arch/x86/pv/emul-mmio-op.c | 166 +
 3 files changed, 167 insertions(+), 129 deletions(-)
 create mode 100644 xen/arch/x86/pv/emul-mmio-op.c

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 3c0aa52f38..a42720c8d1 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4785,11 +4785,6 @@ long arch_memory_op(unsigned long cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 return 0;
 }
 
-
-/*
- * fault handling for read-only MMIO pages
- */
-
 int mmio_ro_emulated_write(
 enum x86_segment seg,
 unsigned long offset,
@@ -4811,130 +4806,6 @@ int mmio_ro_emulated_write(
 return X86EMUL_OKAY;
 }
 
-static const struct x86_emulate_ops mmio_ro_emulate_ops = {
-.read   = x86emul_unhandleable_rw,
-.insn_fetch = pv_emul_ptwr_read,
-.write  = mmio_ro_emulated_write,
-.validate   = pv_emul_is_mem_write,
-.cpuid  = pv_emul_cpuid,
-};
-
-int mmcfg_intercept_write(
-enum x86_segment seg,
-unsigned long offset,
-void *p_data,
-unsigned int bytes,
-struct x86_emulate_ctxt *ctxt)
-{
-struct mmio_ro_emulate_ctxt *mmio_ctxt = ctxt->data;
-
-/*
- * Only allow naturally-aligned stores no wider than 4 bytes to the
- * original %cr2 address.
- */
-if ( ((bytes | offset) & (bytes - 1)) || bytes > 4 || !bytes ||
- offset != mmio_ctxt->cr2 )
-{
-gdprintk(XENLOG_WARNING, "bad write (cr2=%lx, addr=%lx, bytes=%u)\n",
-mmio_ctxt->cr2, offset, bytes);
-return X86EMUL_UNHANDLEABLE;
-}
-
-offset &= 0xfff;
-if ( pci_conf_write_intercept(mmio_ctxt->seg, mmio_ctxt->bdf,
-  offset, bytes, p_data) >= 0 )
-pci_mmcfg_write(mmio_ctxt->seg, PCI_BUS(mmio_ctxt->bdf),
-PCI_DEVFN2(mmio_ctxt->bdf), offset, bytes,
-*(uint32_t *)p_data);
-
-return X86EMUL_OKAY;
-}
-
-static const struct x86_emulate_ops mmcfg_intercept_ops = {
-.read   = x86emul_unhandleable_rw,
-.insn_fetch = pv_emul_ptwr_read,
-.write  = mmcfg_intercept_write,
-.validate   = pv_emul_is_mem_write,
-.cpuid  = pv_emul_cpuid,
-};
-
-/* Check if guest is trying to modify a r/o MMIO page. */
-int mmio_ro_do_page_fault(struct vcpu *v, unsigned long addr,
-  struct cpu_user_regs *regs)
-{
-l1_pgentry_t pte;
-unsigned long mfn;
-unsigned int addr_size = is_pv_32bit_vcpu(v) ? 32 : BITS_PER_LONG;
-struct mmio_ro_emulate_ctxt mmio_ro_ctxt = { .cr2 = addr };
-struct x86_emulate_ctxt ctxt = {
-.regs = regs,
-.vendor = v->domain->arch.cpuid->x86_vendor,
-.addr_size = addr_size,
-.sp_size = addr_size,
-.lma = !is_pv_32bit_vcpu(v),
-.data = &mmio_ro_ctxt,
-};
-int rc;
-
-/* Attempt to read the PTE that maps the VA being accessed. */
-pv_get_guest_eff_l1e(addr, &pte);
-
-/* We are looking only for read-only mappings of MMIO pages. */
-if ( ((l1e_get_flags(pte) & (_PAGE_PRESENT|_PAGE_RW)) != _PAGE_PRESENT) )
-return 0;
-
-mfn = l1e_get_pfn(pte);
-if ( mfn_valid(_mfn(mfn)) )
-{
-struct page_info *page = mfn_to_page(mfn);
-struct domain *owner = page_get_owner_and_reference(page);
-
-if ( owner )
-put_page(page);
-if ( owner != dom_io )
-return 0;
-}
-
-if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn) )
-return 0;
-
-if ( pci_ro_mmcfg_decode(mfn, &mmio_ro_ctxt.seg, &mmio_ro_ctxt.bdf) )
-rc = x86_emulate(&ctxt, &mmcfg_intercept_ops);
-else
-rc = x86_emulate(&ctxt, &mmio_ro_emulate_ops);
-
-switch ( rc )
-{
-case X86EMUL_EXCEPTION:
-/*
- * This emulation only covers writes to MMCFG space or read-only MFNs.
- * We tolerate #PF (from hitting an adjacent page or a successful
- * concurrent pagetable update).  Anything else is an emulation bug,
- * or a guest playing with the instruction stream under Xen's feet.
- */
-if ( ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
- ctxt.event.vector == TRAP_page_fault )
-pv_inject_event(&ctxt.event);
-else
-gdprintk(XENLOG_WARNING,
- "Unexpected event (type %u, vector %#x) from emulation\n",
- ctxt.event.type, ctxt.event.vector);
-
-/* Fallthrough */
-case X86EMUL_OKAY:
-
-if ( ctxt.retire.singlestep )
-pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
-
-/* Fallthrough */
-case X86EMUL_RETRY:
-perfc_in

Re: [Xen-devel] [PATCH v4 12/31] x86/mm: split out readonly MMIO emulation code

2017-08-24 Thread Jan Beulich
>>> On 17.08.17 at 16:44,  wrote:
> Move the code to pv/emul-mmio-op.c. Fix coding style issues while
> moving.
> 
> Note that mmio_ro_emulated_write is needed by both PV and HVM, so it
> is left in x86/mm.c.
> 
> Signed-off-by: Wei Liu 
> ---
>  xen/arch/x86/mm.c  | 129 
>  xen/arch/x86/pv/Makefile   |   1 +
>  xen/arch/x86/pv/emul-mmio-op.c | 166 
> +

Again I think just mmio.c would do. Other comments on earlier
patches apply here as well.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v4 12/31] x86/mm: split out readonly MMIO emulation code

2017-08-24 Thread Andrew Cooper
On 24/08/17 16:16, Jan Beulich wrote:
 On 17.08.17 at 16:44,  wrote:
>> Move the code to pv/emul-mmio-op.c. Fix coding style issues while
>> moving.
>>
>> Note that mmio_ro_emulated_write is needed by both PV and HVM, so it
>> is left in x86/mm.c.
>>
>> Signed-off-by: Wei Liu 
>> ---
>>  xen/arch/x86/mm.c  | 129 
>>  xen/arch/x86/pv/Makefile   |   1 +
>>  xen/arch/x86/pv/emul-mmio-op.c | 166 
>> +
> Again I think just mmio.c would do. Other comments on earlier
> patches apply here as well.

I think it would be wise to merge the ptwr and mmio handling.  At the
moment, we invoke a full lookup pte/decode/try-to-emulate cycle twice in
the #PF handler for PV guests before handing the fault back to the guest.

The correct ops and context can be determined by inspecting the l1e
under %cr2 before calling into any emulation code.

Simplifying this logic before moving it would be the better option.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v4 12/31] x86/mm: split out readonly MMIO emulation code

2017-08-30 Thread Wei Liu
On Thu, Aug 24, 2017 at 04:25:23PM +0100, Andrew Cooper wrote:
> On 24/08/17 16:16, Jan Beulich wrote:
>  On 17.08.17 at 16:44,  wrote:
> >> Move the code to pv/emul-mmio-op.c. Fix coding style issues while
> >> moving.
> >>
> >> Note that mmio_ro_emulated_write is needed by both PV and HVM, so it
> >> is left in x86/mm.c.
> >>
> >> Signed-off-by: Wei Liu 
> >> ---
> >>  xen/arch/x86/mm.c  | 129 
> >>  xen/arch/x86/pv/Makefile   |   1 +
> >>  xen/arch/x86/pv/emul-mmio-op.c | 166 
> >> +
> > Again I think just mmio.c would do. Other comments on earlier
> > patches apply here as well.
> 
> I think it would be wise to merge the ptwr and mmio handling.  At the
> moment, we invoke a full lookup pte/decode/try-to-emulate cycle twice in
> the #PF handler for PV guests before handing the fault back to the guest.
> 
> The correct ops and context can be determined by inspecting the l1e
> under %cr2 before calling into any emulation code.

I will see what I can do. The predicates in traps.c look terribly
complicated.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel