[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v3][PATCH 00/16] Fix RMRR



On 2015/6/11 20:52, Tim Deegan wrote:
Hi,

At 09:15 +0800 on 11 Jun (1434014109), Tiejun Chen wrote:
* Two changes for runtime cycle
    patch #2,xen/x86/p2m: introduce set_identity_p2m_entry, on hypervisor side

   a>. Introduce paging_mode_translate()
   Otherwise, we'll see this error when boot Xen/Dom0

Righto.  Looking at the patch, it would be neater to have

     if ( !paging_mode_translate(p2m->domain) )
         return 0;

at the start, instead of indenting the whole body of the function in
an inner scope.

Right.


   b>. Actually we still need to use "mfn_x(mfn) == INVALID_MFN" to confirm
   we're getting an invalid mfn.

Can you give a concrete example of when this is needed?  The code now

Actually we're considering the case that a VM is set with a big memory like setting "memory = 2800" [0xaf000000] in the .cfg file, but here RMRR is owning such some overlap ranges,

(XEN) [VT-D]dmar.c:808: found ACPI_DMAR_RMRR:
(XEN) [VT-D]dmar.c:677: RMRR region: base_addr ac6d3000 end_address ac6e6fff
(XEN) [VT-D]dmar.c:808: found ACPI_DMAR_RMRR:
(XEN) [VT-D]dmar.c:677: RMRR region: base_addr ad800000 end_address afffffff

Furthermore, our current policy can help us eliminate this sort of conflict. For this example above, the real low memory is limited to 0xacd3000 finally. So indeed, all RMRR regions aren't treated as RAM, and then at least p2m->get_entry() should always return "mfn_x(mfn) == INVALID_MFN", right? And,

looks like this:

        if ( p2mt == p2m_invalid || mfn_x(mfn) == INVALID_MFN )
            ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K,
                                p2m_mmio_direct, p2ma);

which will catch any invalid-mfn mapping, including paged-out pages
&c.   I suspect the case you care about is the default p2m_mmio_dm type,

based on my understanding,

static mfn_t ept_get_entry(struct p2m_domain *p2m,
unsigned long gfn, p2m_type_t *t, p2m_access_t* a,
                           p2m_query_t q, unsigned int *page_order)
{
    ...
    mfn_t mfn = _mfn(INVALID_MFN);


    *t = p2m_mmio_dm;
    ...;

    /* This pfn is higher than the highest the p2m map currently holds */
    if ( gfn > p2m->max_mapped_pfn )
        goto out;
    ...

Actually we're falling into this condition so at last, we're always getting this combination of (*t = p2m_mmio_dm) and (mfn_t mfn = _mfn(INVALID_MFN)).

which would be better handeld explicitly:

        if ( p2mt == p2m_invalid || p2mt == p2m_mmio_dm )
            ...


So if I'm correct, we should do this check explicitly,

       if ( p2mt == p2m_invalid ||
            (p2mt == p2m_mmio_dm && !mfn_valid(mfn) )

Note this is equivalent to Jan's comment.

Thanks
Tiejun

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.