Re: [PATCH v4 13/24] arm/acpi: Map all other tables for Dom0

[Shannon Zhao <shannon.zhao@linaro.org>]

On 2016年03月02日 01:01, Stefano Stabellini wrote:
> On Tue, 1 Mar 2016, Stefano Stabellini wrote:
>> > On Tue, 1 Mar 2016, Shannon Zhao wrote:
>>> > > On 2016/2/29 22:15, Stefano Stabellini wrote:
>>>> > > > On Sun, 28 Feb 2016, Shannon Zhao wrote:
>>>>>> > > >> > From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>> > > >> > 
>>>>>> > > >> > Map all other tables to Dom0 using 1:1 mappings.
>>>>>> > > >> > 
>>>>>> > > >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>> > > >> > ---
>>>>>> > > >> > v4: fix commit message
>>>>>> > > >> > ---
>>>>>> > > >> >  xen/arch/arm/domain_build.c | 26 ++++++++++++++++++++++++++
>>>>>> > > >> >  1 file changed, 26 insertions(+)
>>>>>> > > >> > 
>>>>>> > > >> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>>>> > > >> > index 64e48ae..6ad420c 100644
>>>>>> > > >> > --- a/xen/arch/arm/domain_build.c
>>>>>> > > >> > +++ b/xen/arch/arm/domain_build.c
>>>>>> > > >> > @@ -1357,6 +1357,30 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>>>>>> > > >> >  }
>>>>>> > > >> >  
>>>>>> > > >> >  #ifdef CONFIG_ACPI
>>>>>> > > >> > +static void acpi_map_other_tables(struct domain *d)
>>>>>> > > >> > +{
>>>>>> > > >> > +    int i;
>>>>>> > > >> > +    unsigned long res;
>>>>>> > > >> > +    u64 addr, size;
>>>>>> > > >> > +
>>>>>> > > >> > +    /* Map all other tables to Dom0 using 1:1 mappings. */
>>>>>> > > >> > +    for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
>>>>>> > > >> > +    {
>>>>>> > > >> > +        addr = acpi_gbl_root_table_list.tables[i].address;
>>>>>> > > >> > +        size = acpi_gbl_root_table_list.tables[i].length;
>>>>>> > > >> > +        res = map_regions(d,
>>>>>> > > >> > +                          paddr_to_pfn(addr & PAGE_MASK),
>>>>>> > > >> > +                          DIV_ROUND_UP(size, PAGE_SIZE),
>>>>>> > > >> > +                          paddr_to_pfn(addr & PAGE_MASK));
>>>>>> > > >> > +        if ( res )
>>>>>> > > >> > +        {
>>>>>> > > >> > +             panic(XENLOG_ERR "Unable to map 0x%"PRIx64
>>>>>> > > >> > +                   " - 0x%"PRIx64" in domain \n",
>>>>>> > > >> > +                   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
>>>>>> > > >> > +        }
>>>>>> > > >> > +    }
>>>>>> > > >> > +}
>>>> > > > The problem with this function is that it is mapping all other tables to
>>>> > > > the guest, including the unmodified FADT and MADT. This way dom0 is
>>>> > > > going to find two different versions of the FADT and MADT, isn't that
>>>> > > > right?
>>>> > > >  
>>> > > We've replaced the entries of XSDT table with new value. That means XSDT
>>> > > points to new table. Guest will not see the old ones.
>> > 
>> > All right. Of course it would be best to avoid mapping the original FADT
>> > and MADT at all, but given that they are not likely to be page aligned,
>> > it wouldn't be easy to do.
>> > 
>> > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> However I have one more question: given that map_regions maps the memory
> read-only to Dom0, isn't it possible that one or more of the DSDT
> functions could not work as expected? I would imagine that the ACPI
> bytecode is allowed to change its own memory, right?
> 
I'm not sure about this. But it seems that Xen or Linux always map these
tables to its memory.

Thanks,
--
Shannon

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