xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Mukesh Rathor <mukesh.rathor@oracle.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: George.Dunlap@eu.citrix.com, xen-devel@lists.xenproject.org,
	keir.xen@gmail.com, tim@xen.org, JBeulich@suse.com
Subject: Re: [V10 PATCH 0/4] pvh dom0 patches...
Date: Mon, 5 May 2014 17:28:58 -0700	[thread overview]
Message-ID: <20140505172858.4c6c3f0a@mantra.us.oracle.com> (raw)
In-Reply-To: <53675166.30400@citrix.com>

On Mon, 5 May 2014 10:52:54 +0200
Roger Pau Monné <roger.pau@citrix.com> wrote:

> On 03/05/14 02:01, Mukesh Rathor wrote:
> > On Fri, 2 May 2014 13:05:23 +0200
> > Roger Pau Monné <roger.pau@citrix.com> wrote:
> > 
> >> On 01/05/14 03:19, Mukesh Rathor wrote:
> >>> On Wed, 30 Apr 2014 11:12:16 -0700
> >>> Mukesh Rathor <mukesh.rathor@oracle.com> wrote:
> >>>
> >>>> On Wed, 30 Apr 2014 16:11:39 +0200
> >>>> Roger Pau Monné <roger.pau@citrix.com> wrote:
> >>>>
> >>>>> On 30/04/14 03:06, Mukesh Rathor wrote:
> >>>> .....
> >>>>
> >>>>> Hello Mukesh,
> >>>>>
.......
> >> With the patch applied I can boot fine, no error messages at all.
> >> I've printed the address that's causing the vioapic_range call,
> >> it's 0x1073741824, which according to the e820 map passed by Xen
> >> falls into a region marked as valid memory:
> >>
> >> SMAP type=01 base=0000000000100000 len=000000003ff6e000
> >>
> >> The crash happens because FreeBSD scrubs all valid memory at early
> >> boot when booted with hw.memtest.tests=1.
> > 
> > Hi Roger,
> 
> Hello Mukesh, thanks for the help.
> 
> > I think something else is going on here. 
> > The vioapic address check is fenced by is_hvm check, 
> > 
> >     if ( !nestedhvm_vcpu_in_guestmode(v)
> >          && is_hvm_vcpu(v)    <====
> >          && gfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v))) )
> >     {
> 
> AFAIK this is not the path that's causing the fault, the fault comes
> from:
> 
>     if ( (p2mt == p2m_mmio_dm) ||
>          (access_w && (p2mt == p2m_ram_ro)) )
>     {
>         put_gfn(p2m->domain, gfn);
>         if ( !handle_mmio() ) <=====
>             hvm_inject_hw_exception(TRAP_gp_fault, 0);
>         rc = 1;
>         goto out;
>     }
> 
> This was happening because I was trying to access a gpfn from outside
> of the p2m map, which didn't have a valid mfn. The type of the page
> was p2m_mmio_dm, the access p2m_access_n and the mfn was not valid
> (I've done a p2m->get_entry on the faulting address).

Ok, I know what's going on. By default, the p2m type returned is
p2m_mmio_dm. I'll resubmit my vioapic patch. But, your real issue
here is pages released from punched holes not added back. See below
for that. Once you fix that, you'll not see this, unless some other
kernel bug causing ept violation.

> This was because I was using start_info->nr_pages as the number of
> usable RAM pages, but AFAICT from the code in domain_build.c,
> pvh_map_all_iomem is making holes in the p2m, but it is not adding
> those freed pages back to the end of the memory map, so the value in
> nr_pages is not the number of usable RAM pages, but the number of
> pages in the p2m map (taking into account both usable RAM pages and
> p2m_mmio_direct regions).
> 
> I'm not sure if this logic is correct, shouldn't the freed pages by
> pvh_map_all_iomem be added to the end of the memory map?

Yeah, it's confusing a bit. Let me talk in terms of linux.

In case of PV dom0, linux parses the e820 and punches holes in
the p2m itself. See xen_set_identity_and_release(). For pvh, we
can skip some of that (since it already happened in xen), but we still 
use the "released" variable to keep track of that. Then later, 
xen_populate_chunk() adds those "released" pages back via 
XENMEM_populate_physmap. This happens for both PV and PVH, so the 
pages are added back. 

xen_memory_setup():
        /*
         * Populate back the non-RAM pages and E820 gaps that had been
         * released. */
        populated = xen_populate_chunk(map, memmap.nr_entries,
                        max_pfn, &last_pfn, xen_released_pages);


Perhaps your logic that does similar for PV needs to make sure it is 
populating the pages back for pvh also?  You just don't need to 
punch holes in the p2m as you do for PV, for PVH you can skip that 
part.  Hope that makes sense.

FWIW, my very first patch didn't do that in xen, but was done in linux
same as for PV. This required a new hypercall. But several maintainers 
felt we should map all iomem in xen upfront.

thanks
mukesh


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2014-05-06  0:29 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-30  1:06 [V10 PATCH 0/4] pvh dom0 patches Mukesh Rathor
2014-04-30  1:06 ` [V10 PATCH 1/4] pvh dom0: construct_dom0 changes Mukesh Rathor
2014-05-06 15:18   ` Roger Pau Monné
2014-04-30  1:06 ` [V10 PATCH 2/4] pvh dom0: Add checks and restrictions for p2m_is_foreign Mukesh Rathor
2014-05-01 16:14   ` Tim Deegan
2014-04-30  1:06 ` [V10 PATCH 3/4] pvh dom0: Add and remove foreign pages Mukesh Rathor
2014-05-01 16:19   ` Tim Deegan
2014-05-02  1:45     ` Mukesh Rathor
2014-05-02  8:38       ` Jan Beulich
2014-05-02  8:55       ` Tim Deegan
2014-05-02 23:35         ` Mukesh Rathor
2014-05-05  7:46           ` Jan Beulich
2014-05-08 12:16           ` Tim Deegan
2014-05-08 13:25             ` Jan Beulich
2014-05-08 22:58             ` Mukesh Rathor
2014-04-30  1:06 ` [V10 PATCH 4/4] dom0: add opt_dom0pvh to setup.c Mukesh Rathor
2014-04-30 14:11 ` [V10 PATCH 0/4] pvh dom0 patches Roger Pau Monné
2014-04-30 18:12   ` Mukesh Rathor
2014-05-01  1:19     ` Mukesh Rathor
2014-05-02 11:05       ` Roger Pau Monné
2014-05-02 12:31         ` Jan Beulich
2014-05-02 14:06           ` Roger Pau Monné
2014-05-02 14:16             ` Jan Beulich
2014-05-02 14:35               ` Roger Pau Monné
2014-05-02 15:41                 ` Jan Beulich
2014-05-02 16:13                   ` Roger Pau Monné
2014-05-02 19:35                     ` Konrad Rzeszutek Wilk
2014-05-03  0:01         ` Mukesh Rathor
2014-05-05  8:52           ` Roger Pau Monné
2014-05-06  0:28             ` Mukesh Rathor [this message]
2014-05-06  7:13               ` Roger Pau Monné
2014-05-06  8:09                 ` Jan Beulich
2014-05-07  1:00                 ` Mukesh Rathor
2014-05-07  7:50                   ` Jan Beulich
2014-05-07  9:48                     ` Roger Pau Monné
2014-05-07 11:34                       ` Jan Beulich
2014-05-08 10:27                         ` Roger Pau Monné
2014-05-08 10:44                           ` Jan Beulich
2014-05-08 15:00                             ` Roger Pau Monné
2014-05-08 15:20                               ` Jan Beulich
2014-05-07 13:25                     ` Konrad Rzeszutek Wilk
2014-05-08  0:04                     ` Mukesh Rathor
2014-05-08  6:37                       ` Jan Beulich
2014-05-08 19:15                         ` Mukesh Rathor
2014-05-07 13:20                   ` Konrad Rzeszutek Wilk
2014-05-07 13:38                     ` Roger Pau Monné
2014-05-08  0:12                       ` Mukesh Rathor
2014-05-08 10:52                         ` George Dunlap
2014-05-08 13:15                         ` David Vrabel
2014-05-08 22:29                           ` Mukesh Rathor
2014-05-08  0:07                     ` Mukesh Rathor
2014-05-06 19:38               ` Konrad Rzeszutek Wilk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140505172858.4c6c3f0a@mantra.us.oracle.com \
    --to=mukesh.rathor@oracle.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=keir.xen@gmail.com \
    --cc=roger.pau@citrix.com \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).