|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5/6] xen/arm: map reserved-memory regions as normal memory in dom0
On Tue, 26 Feb 2019, Julien Grall wrote:
> Hi Stefano,
>
> On 26/02/2019 23:07, Stefano Stabellini wrote:
> > reserved-memory regions should be mapped as normal memory. At the
> > moment, they get remapped as device memory in dom0 because Xen doesn't
> > know any better. Add an explicit check for it.
>
> You probably use an outdated change (> 2 years ago). In recent Xen, Dom0
> MMIO are mapped use p2m_mmio_direct_c. This main difference with
> p2m_ram_rw is the shareability attribute (inner vs outer).
>
> This will also have the advantage to not impair with the rest of Xen.
I have already fixed this in my tree.
> But I don't think this would be enough. Per [1], reserved-memory region
> is used to carve memory from /memory node. So those regions should be
> described in /memory of the Dom0 DT as well.
>
> >
> > However, reserved-memory regions are allowed to overlap partially or
> > completely with memory nodes. In these cases, the overlapping memory is
> > reserved-memory and should be handled accordingly.
>
> Do you mind providing your source? If you look at the description in
> Linux bindings, it is clearly they will always overlap with /memory.
You are right. Reserved-memory regions have to fully overlap with
/memory, and that assumption can simplify the implementation.
> [...]
>
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 80f0028..74c4707 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -470,10 +470,52 @@ static void __init init_pdx(void)
> > }
> > }
> >
> > +static void __init check_reserved_memory(paddr_t *bank_start, paddr_t
> > *bank_size)
> > +{
> > + paddr_t bank_end = *bank_start + *bank_size;
> > + struct meminfo mem = bootinfo.mem;
> > + int i;
> > +
> > + for ( i = 0; i < bootinfo.reserved_mem.nr_banks; i++ )
> > + {
> > + struct membank rbank = bootinfo.reserved_mem.bank[i];
> > +
> > + if ( *bank_start < rbank.start && bank_end <= rbank.start )
> > + continue;
> > +
> > + if ( *bank_start >= (rbank.start + rbank.size) )
> > + continue;
> > +
> > + /* memory bank overlaps with reserved memory region */
> > + if ( rbank.start > *bank_start )
> > + {
> > + bank_end = rbank.start;
> > + if ( *bank_start + *bank_size > rbank.start + rbank.size )
> > + {
> > + mem.bank[mem.nr_banks].start = rbank.start + rbank.size;
> > + mem.bank[mem.nr_banks].size = *bank_start + *bank_size -
> > + mem.bank[mem.nr_banks].start;
> > + mem.nr_banks++;
> > + }
> > + }
> > + else if ( rbank.start + rbank.size > *bank_start)
> > + {
> > + if (rbank.start + rbank.size < bank_end )
> > + *bank_start = rbank.start + rbank.size;
> > + else
> > + *bank_start = bank_end;
> > + }
> > +
> > + *bank_size = bank_end - *bank_start;
> > + }
> > +}
>
> reserved-memory nodes is more nothing more than an extension of an old
> DT binding for reserved memory. We handle them in a few places (see
> consider_modules and dt_unreserved_region). So mostly likely you want to
> extend what we already have.
>
> This would avoid most (if not) all the changes below.
I take your point that this code could be simplified because
reserved-memory has to be described under /memory too. I can do that.
I am not sure about the suggestion of re-using the libfdt concept of
"mem_rsv", which is meant to be for the old /memreserve/. Today, libfdt
(at least our version of it) is not able to parse the new
reserved-memory bindings. I don't think it is a good idea to modify
libfdt for that. Also, the way we want to handle the old memreserve
regions is quite different from the way we want to handle
reserved-memory, right? I cannot see a way to improve this code using
mem_rsv at the moment.
> > +
> > #ifdef CONFIG_ARM_32
> > static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
> > {
> > - paddr_t ram_start, ram_end, ram_size;
> > + paddr_t ram_start = ~0;
> > + paddr_t ram_end = 0;
> > + paddr_t ram_size = 0;
> > paddr_t s, e;
> > unsigned long ram_pages;
> > unsigned long heap_pages, xenheap_pages, domheap_pages;
> > @@ -487,18 +529,19 @@ static void __init setup_mm(unsigned long dtb_paddr,
> > size_t dtb_size)
> >
> > init_pdx();
> >
> > - ram_start = bootinfo.mem.bank[0].start;
> > - ram_size = bootinfo.mem.bank[0].size;
> > - ram_end = ram_start + ram_size;
> > -
> > - for ( i = 1; i < bootinfo.mem.nr_banks; i++ )
> > + for ( i = 0; i < bootinfo.mem.nr_banks; i++ )
> > {
> > - paddr_t bank_start = bootinfo.mem.bank[i].start;
> > - paddr_t bank_size = bootinfo.mem.bank[i].size;
> > - paddr_t bank_end = bank_start + bank_size;
> > + paddr_t bank_end;
> >
> > - ram_size = ram_size + bank_size;
> > - ram_start = min(ram_start,bank_start);
> > + check_reserved_memory(&bootinfo.mem.bank[i].start,
> > + &bootinfo.mem.bank[i].size);
> > +
> > + if ( !bootinfo.mem.bank[i].size )
> > + continue;
> > +
> > + bank_end = bootinfo.mem.bank[i].start + bootinfo.mem.bank[i].size;
> > + ram_size = ram_size + bootinfo.mem.bank[i].size;
> > + ram_start = min(ram_start, bootinfo.mem.bank[i].start);
> > ram_end = max(ram_end,bank_end);
> > }
> >
> > @@ -570,6 +613,9 @@ static void __init setup_mm(unsigned long dtb_paddr,
> > size_t dtb_size)
> > paddr_t bank_start = bootinfo.mem.bank[i].start;
> > paddr_t bank_end = bank_start + bootinfo.mem.bank[i].size;
> >
> > + if ( !bootinfo.mem.bank[i].size )
> > + continue;
> > +
> > s = bank_start;
> > while ( s < bank_end )
> > {
> > @@ -627,11 +673,21 @@ static void __init setup_mm(unsigned long dtb_paddr,
> > size_t dtb_size)
> > total_pages = 0;
> > for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
> > {
> > - paddr_t bank_start = bootinfo.mem.bank[bank].start;
> > - paddr_t bank_size = bootinfo.mem.bank[bank].size;
> > - paddr_t bank_end = bank_start + bank_size;
> > + paddr_t bank_start;
> > + paddr_t bank_size;
> > + paddr_t bank_end;
> > paddr_t s, e;
> >
> > + check_reserved_memory(&bootinfo.mem.bank[bank].start,
> > + &bootinfo.mem.bank[bank].size);
> > +
> > + bank_start = bootinfo.mem.bank[bank].start;
> > + bank_size = bootinfo.mem.bank[bank].size;
> > + bank_end = bank_start + bank_size;
> > +
> > + if ( !bank_size )
> > + continue;
> > +
> > ram_size = ram_size + bank_size;
> > ram_start = min(ram_start,bank_start);
> > ram_end = max(ram_end,bank_end);
> >
>
> [1]
> https://www.kernel.org/doc/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |