| Message ID | 1431893048-5214-33-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 8 June 2015 at 22:20, Julien Grall <julien.grall@citrix.com> wrote: > Hi Parth, > > On 17/05/2015 21:03, Parth Dixit wrote: >> >> In ACPI mmio regions are described in DSDT which requires >> AML interpreter. Defer the mapping of mmio until DSDT is parsed in >> DOM0 and map mmio's dynamically at the time of request. > > > I'm against a such solution. Even though it's DOM0 we should avoid to allow > him to map anything (RAM,...) on data abort. I think we agreed to this solution http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html > During DOM0 creation, we should map anything that is not RAM and not used by > Xen to DOM0. IIRC, this is how it works on x86. > > I'm nearly sure we talked about it during the Linaro Connect. Please give > details if you thing it won't work... > > Regards, > > -- > Julien Grall
+shannon On 15 June 2015 at 06:49, Julien Grall <julien.grall@citrix.com> wrote: > Hi Parth, > > On 14/06/2015 11:27, Parth Dixit wrote: >> >> On 8 June 2015 at 22:20, Julien Grall <julien.grall@citrix.com> wrote: >>> >>> Hi Parth, >>> >>> On 17/05/2015 21:03, Parth Dixit wrote: >>>> >>>> >>>> In ACPI mmio regions are described in DSDT which requires >>>> AML interpreter. Defer the mapping of mmio until DSDT is parsed in >>>> DOM0 and map mmio's dynamically at the time of request. >>> >>> >>> >>> I'm against a such solution. Even though it's DOM0 we should avoid to >>> allow >>> him to map anything (RAM,...) on data abort. >> >> I think we agreed to this solution >> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html > > > Firstly, this kind of link should have been put in the changelog of the > patch (after ---). It helps the reviewer to know what was decided (or not) > on the previous discussion. It's more true with a series of more than 40 > patches... > > Secondly, the thread you pointed as some discussion on it but no formal > agreement about what to do. If there is no answer, it's better to do a > resume and ask if anyone are agree. > > Finally, what you've implemented and what was suggested by Ian is different. > You are allowing any region to be mapped in DOM0 via this way. Only MMIO > should be allowed. > > Concerning the mapping attribute used. Based on Ard answer "The UEFI spec > mandates that the memory map describes all memory in the system, so if dom0 > accesses any ranges outside of that, it makes sense > to just use device mappings for stage 2.". We should use by default Device > Stage 2, it's safer. If it doesn't work later (because some PCI BAR may be > memory, which if I wasn't able to prove), then we can think differently. > > For the mapping of the MMIO to DOM0, I believe we can map any non-RAM (and > any non-RAM not used by Xen) regions to DOM0 at boot time (I think x86 does > that). It would keep the ACPI code contained at boot time and no difference > during runtime. > > Regards, > > -- > Julien Grall
On 2015/7/5 21:30, Parth Dixit wrote: > +shannon > > On 15 June 2015 at 06:49, Julien Grall <julien.grall@citrix.com> wrote: >> Hi Parth, >> >> On 14/06/2015 11:27, Parth Dixit wrote: >>> >>> On 8 June 2015 at 22:20, Julien Grall <julien.grall@citrix.com> wrote: >>>> >>>> Hi Parth, >>>> >>>> On 17/05/2015 21:03, Parth Dixit wrote: >>>>> >>>>> >>>>> In ACPI mmio regions are described in DSDT which requires >>>>> AML interpreter. Defer the mapping of mmio until DSDT is parsed in >>>>> DOM0 and map mmio's dynamically at the time of request. >>>> >>>> >>>> >>>> I'm against a such solution. Even though it's DOM0 we should avoid to >>>> allow >>>> him to map anything (RAM,...) on data abort. >>> >>> I think we agreed to this solution >>> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html >> >> >> Firstly, this kind of link should have been put in the changelog of the >> patch (after ---). It helps the reviewer to know what was decided (or not) >> on the previous discussion. It's more true with a series of more than 40 >> patches... >> >> Secondly, the thread you pointed as some discussion on it but no formal >> agreement about what to do. If there is no answer, it's better to do a >> resume and ask if anyone are agree. >> >> Finally, what you've implemented and what was suggested by Ian is different. >> You are allowing any region to be mapped in DOM0 via this way. Only MMIO >> should be allowed. >> >> Concerning the mapping attribute used. Based on Ard answer "The UEFI spec >> mandates that the memory map describes all memory in the system, so if dom0 >> accesses any ranges outside of that, it makes sense >> to just use device mappings for stage 2.". We should use by default Device >> Stage 2, it's safer. If it doesn't work later (because some PCI BAR may be >> memory, which if I wasn't able to prove), then we can think differently. >> >> For the mapping of the MMIO to DOM0, I believe we can map any non-RAM (and >> any non-RAM not used by Xen) regions to DOM0 at boot time (I think x86 does >> that). It would keep the ACPI code contained at boot time and no difference >> during runtime. >> But How do we get the MMIO information of the devices in DSDT table? If we want to parse DSDT table, we must introduce AML interpreter, IIUC. Julien, do you have any other ideas? Thanks,
Hi Shannon,
instead of getting mmio information for individual
devices from dsdt, we will map all the non-ram regions described in
uefi. AML interpreter option was discussed earlier and it was decided
not to go with that approach. You can find more details in the linaro
xen wiki for the reasoning behind it.
regards
parth
On 30 July 2015 at 17:49, Shannon Zhao <shannon.zhao@linaro.org> wrote:
>
>
> On 2015/7/5 21:30, Parth Dixit wrote:
>> +shannon
>>
>> On 15 June 2015 at 06:49, Julien Grall <julien.grall@citrix.com> wrote:
>>> Hi Parth,
>>>
>>> On 14/06/2015 11:27, Parth Dixit wrote:
>>>>
>>>> On 8 June 2015 at 22:20, Julien Grall <julien.grall@citrix.com> wrote:
>>>>>
>>>>> Hi Parth,
>>>>>
>>>>> On 17/05/2015 21:03, Parth Dixit wrote:
>>>>>>
>>>>>>
>>>>>> In ACPI mmio regions are described in DSDT which requires
>>>>>> AML interpreter. Defer the mapping of mmio until DSDT is parsed in
>>>>>> DOM0 and map mmio's dynamically at the time of request.
>>>>>
>>>>>
>>>>>
>>>>> I'm against a such solution. Even though it's DOM0 we should avoid to
>>>>> allow
>>>>> him to map anything (RAM,...) on data abort.
>>>>
>>>> I think we agreed to this solution
>>>> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html
>>>
>>>
>>> Firstly, this kind of link should have been put in the changelog of the
>>> patch (after ---). It helps the reviewer to know what was decided (or not)
>>> on the previous discussion. It's more true with a series of more than 40
>>> patches...
>>>
>>> Secondly, the thread you pointed as some discussion on it but no formal
>>> agreement about what to do. If there is no answer, it's better to do a
>>> resume and ask if anyone are agree.
>>>
>>> Finally, what you've implemented and what was suggested by Ian is different.
>>> You are allowing any region to be mapped in DOM0 via this way. Only MMIO
>>> should be allowed.
>>>
>>> Concerning the mapping attribute used. Based on Ard answer "The UEFI spec
>>> mandates that the memory map describes all memory in the system, so if dom0
>>> accesses any ranges outside of that, it makes sense
>>> to just use device mappings for stage 2.". We should use by default Device
>>> Stage 2, it's safer. If it doesn't work later (because some PCI BAR may be
>>> memory, which if I wasn't able to prove), then we can think differently.
>>>
>>> For the mapping of the MMIO to DOM0, I believe we can map any non-RAM (and
>>> any non-RAM not used by Xen) regions to DOM0 at boot time (I think x86 does
>>> that). It would keep the ACPI code contained at boot time and no difference
>>> during runtime.
>>>
>
> But How do we get the MMIO information of the devices in DSDT table? If
> we want to parse DSDT table, we must introduce AML interpreter, IIUC.
>
> Julien, do you have any other ideas?
>
> Thanks,
> --
> Shannon
On 31 July 2015 at 00:01, Julien Grall <julien.grall@citrix.com> wrote: > Hi, > > On 30/07/15 19:02, Parth Dixit wrote: >> instead of getting mmio information for individual >> devices from dsdt, we will map all the non-ram regions described in >> uefi. AML interpreter option was discussed earlier and it was decided >> not to go with that approach. You can find more details in the linaro >> xen wiki for the reasoning behind it. > > Which page are you talking about? I only found [1] speaking about ACPI. > Although, there is nothing related to MMIO mapping. I was talking about the reasons for not using aml interpreter in xen. > Anyway, it's not possible to get the list of MMIOs regions for the UEFI > System Memory Map (see the mail you forward on the ML [2]). > Although, based on the RAM region we could deduce a possible set of MMIO > regions. It would be fine to mapped unused region in memory and we could > take advantage of super page. > > While you are speaking about the wiki page. Can one of you update the > wiki page about the boot protocol? Jan had some concerns about the > solution you choose (see [3] to not mention the whole thread). > > We need to agree on the boot protocol before going further on this series. > > To speed up the upstreaming process, it would be nice if you start a > thread about the boot protocol for ACPI with relevant people in CCed. > The main goal will be to explain why you choose this way. This will be > the base to talk about improvement and/or answer concerns for other people. > > FWIW, Jan also suggested a different boot protocol based on the x86 one. > It may be good for you to see whether it would fit ACPI on ARM. > > Regards, > > [1] https://wiki.linaro.org/LEG/Engineering/Virtualization/ACPI_on_Xen > > [2] > http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html > > [3] http://lists.xen.org/archives/html/xen-devel/2015-05/msg02736.html > > -- > Julien Grall
On 2015/7/31 2:02, Parth Dixit wrote: > Hi Shannon, > instead of getting mmio information for individual > devices from dsdt, we will map all the non-ram regions described in > uefi. Yes, I don't want to use AML interpreter either. But how to get all the non-ram regions? > AML interpreter option was discussed earlier and it was decided > not to go with that approach. You can find more details in the linaro > xen wiki for the reasoning behind it. > > regards > parth > > On 30 July 2015 at 17:49, Shannon Zhao <shannon.zhao@linaro.org> wrote: >> >> >> On 2015/7/5 21:30, Parth Dixit wrote: >>> +shannon >>> >>> On 15 June 2015 at 06:49, Julien Grall <julien.grall@citrix.com> wrote: >>>> Hi Parth, >>>> >>>> On 14/06/2015 11:27, Parth Dixit wrote: >>>>> >>>>> On 8 June 2015 at 22:20, Julien Grall <julien.grall@citrix.com> wrote: >>>>>> >>>>>> Hi Parth, >>>>>> >>>>>> On 17/05/2015 21:03, Parth Dixit wrote: >>>>>>> >>>>>>> >>>>>>> In ACPI mmio regions are described in DSDT which requires >>>>>>> AML interpreter. Defer the mapping of mmio until DSDT is parsed in >>>>>>> DOM0 and map mmio's dynamically at the time of request. >>>>>> >>>>>> >>>>>> >>>>>> I'm against a such solution. Even though it's DOM0 we should avoid to >>>>>> allow >>>>>> him to map anything (RAM,...) on data abort. >>>>> >>>>> I think we agreed to this solution >>>>> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html >>>> >>>> >>>> Firstly, this kind of link should have been put in the changelog of the >>>> patch (after ---). It helps the reviewer to know what was decided (or not) >>>> on the previous discussion. It's more true with a series of more than 40 >>>> patches... >>>> >>>> Secondly, the thread you pointed as some discussion on it but no formal >>>> agreement about what to do. If there is no answer, it's better to do a >>>> resume and ask if anyone are agree. >>>> >>>> Finally, what you've implemented and what was suggested by Ian is different. >>>> You are allowing any region to be mapped in DOM0 via this way. Only MMIO >>>> should be allowed. >>>> >>>> Concerning the mapping attribute used. Based on Ard answer "The UEFI spec >>>> mandates that the memory map describes all memory in the system, so if dom0 >>>> accesses any ranges outside of that, it makes sense >>>> to just use device mappings for stage 2.". We should use by default Device >>>> Stage 2, it's safer. If it doesn't work later (because some PCI BAR may be >>>> memory, which if I wasn't able to prove), then we can think differently. >>>> >>>> For the mapping of the MMIO to DOM0, I believe we can map any non-RAM (and >>>> any non-RAM not used by Xen) regions to DOM0 at boot time (I think x86 does >>>> that). It would keep the ACPI code contained at boot time and no difference >>>> during runtime. >>>> >> >> But How do we get the MMIO information of the devices in DSDT table? If >> we want to parse DSDT table, we must introduce AML interpreter, IIUC. >> >> Julien, do you have any other ideas? >> >> Thanks, >> -- >> Shannon
On 2015/7/31 2:31, Julien Grall wrote: > Hi, > > On 30/07/15 19:02, Parth Dixit wrote: >> instead of getting mmio information for individual >> devices from dsdt, we will map all the non-ram regions described in >> uefi. AML interpreter option was discussed earlier and it was decided >> not to go with that approach. You can find more details in the linaro >> xen wiki for the reasoning behind it. > > Which page are you talking about? I only found [1] speaking about ACPI. > Although, there is nothing related to MMIO mapping. > > Anyway, it's not possible to get the list of MMIOs regions for the UEFI > System Memory Map (see the mail you forward on the ML [2]). > > Although, based on the RAM region we could deduce a possible set of MMIO > regions. But I guess this will get the all regions except RAM region. And some of the regions may not exist on hardware. Is it ok to map the non-exist region to DOM0? Doesn't the map function fail? > It would be fine to mapped unused region in memory and we could > take advantage of super page. > > While you are speaking about the wiki page. Can one of you update the > wiki page about the boot protocol? Jan had some concerns about the > solution you choose (see [3] to not mention the whole thread). > About the XENV table, from the discussions of the thread, it seems we reach an agreement on using hypercall to tell DOM0 the grant table info and event channel irq. Right? > We need to agree on the boot protocol before going further on this series. > > To speed up the upstreaming process, it would be nice if you start a > thread about the boot protocol for ACPI with relevant people in CCed. > The main goal will be to explain why you choose this way. This will be > the base to talk about improvement and/or answer concerns for other people. > Ok, it's good to reach an agreement before action. > FWIW, Jan also suggested a different boot protocol based on the x86 one. > It may be good for you to see whether it would fit ACPI on ARM. > Which boot protocol? Could you point it out? Thanks. :) > Regards, > > [1] https://wiki.linaro.org/LEG/Engineering/Virtualization/ACPI_on_Xen > > [2] > http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html > > [3] http://lists.xen.org/archives/html/xen-devel/2015-05/msg02736.html >
On Fri, Jul 31, 2015 at 4:09 PM, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: [...] > > > One option going forward is to map MMIO regions in Dom0 on demand when > trapping in Xen with a data abort. Specifically in > xen/arch/arm/traps.c:do_trap_data_abort_guest we could check that the > guest is dom0 and that the address correspond to a non-ram region not > owned by Xen. If the checks succeed then we map the page in Dom0. > > I seem to remember people raising issues about mapping device memory on-demand on KVM/arm64, because you can have weird timing issues happening if a register map spans multiple pages etc., but i can't seem to find the original e-mail. I think it was Ard/Marc/Peter discussing this. [...] >> >> >> > >> > Which boot protocol? Could you point it out? Thanks. :) >> >> The way to boot DOM0 with ACPI. There is a page on the Linaro wiki [1], >> but the content is quite out of date now. >> >> Regards, >> >> [1] https://wiki.linaro.org/LEG/Engineering/Virtualization/ACPI_on_Xen > > http://marc.info/?i=1431893048-5214-1-git-send-email-parth.dixit%40linaro.org > is a good start, but it needs more details. The important thing to clear > out is which information is passed to Dom0 and how, because it will > become a supported external interface going forward. > > Specifically: > - what information is passed via the small device tree to dom0 and in > what format > - how the acpi tables are given to dom0 > * mapped or copied? > * how do we pass a pointer to them to the kernel? > - if some tables are changed by Xen before passing them on, it would be > good to list what was changed > * what tables have been modified > * what tables have been added > * what tables have been removed > - how is the memory map passed to Dom0 > * how do we find out the list of MMIO regions, both temporary and > future solutions > * how to we tell dom0 where they are Also, is there going to be some paravirtualized interface for the info that systems would normally get from UEFI, or are we going to emulate UEFI, or is all this going to be in the small DT? (I know this has been discussed before, but I have forgotten the conclusions) -Christoffer
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 47d6cef..6b8d247 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -31,6 +31,7 @@ #include <xen/softirq.h> #include <xen/domain_page.h> #include <xen/perfc.h> +#include <xen/acpi.h> #include <public/sched.h> #include <public/xen.h> #include <asm/debugger.h> @@ -46,6 +47,7 @@ #include "vtimer.h" #include <asm/gic.h> #include <asm/vgic.h> +#include <xen/iocap.h> /* The base of the stack must always be double-word aligned, which means * that both the kernel half of struct cpu_user_regs (which is pushed in @@ -2336,6 +2338,47 @@ bad_insn_abort: inject_iabt_exception(regs, gva, hsr.len); } +#ifdef CONFIG_ACPI +static int map_mmio_dsdt(struct cpu_user_regs *regs, + mmio_info_t *info) +{ + int res; + u64 addr,size; + struct domain* d = current->domain; + + if ( !is_hardware_domain(d) ) + return 0; + + addr = info->gpa; + size = PAGE_SIZE; + + res = iomem_permit_access(d, paddr_to_pfn(addr & PAGE_MASK), + paddr_to_pfn(PAGE_ALIGN(addr + size - 1))); + if ( res ) + { + printk(XENLOG_ERR "Unable to permit to dom%d access to" + " 0x%"PRIx64" - 0x%"PRIx64"\n", + d->domain_id, + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1); + return 0; + } + res = map_mmio_regions(d, + paddr_to_pfn(addr & PAGE_MASK), + DIV_ROUND_UP(size, PAGE_SIZE), + paddr_to_pfn(addr & PAGE_MASK)); + if ( res ) + { + printk(XENLOG_ERR "Unable to map 0x%"PRIx64 + " - 0x%"PRIx64" in domain %d\n", + addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1, + d->domain_id); + return 0; + } + + return 1; +} +#endif + static void do_trap_data_abort_guest(struct cpu_user_regs *regs, const union hsr hsr) { @@ -2411,7 +2454,13 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs, advance_pc(regs, hsr); return; } - +#ifdef CONFIG_ACPI + if( !acpi_disabled ) + { + if( map_mmio_dsdt(regs, &info) ) + return; + } +#endif bad_data_abort: inject_dabt_exception(regs, info.gva, hsr.len); }
In ACPI mmio regions are described in DSDT which requires AML interpreter. Defer the mapping of mmio until DSDT is parsed in DOM0 and map mmio's dynamically at the time of request. Signed-off-by: Parth Dixit <parth.dixit@linaro.org> --- xen/arch/arm/traps.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-)