diff mbox

[Xen-devel,v6,13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0

Message ID 1458207668-12012-14-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao March 17, 2016, 9:40 a.m. UTC 
From: Shannon Zhao <shannon.zhao@linaro.org>

Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Julien Grall March 22, 2016, 12:42 a.m. UTC | #1 
Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>   xen/arch/arm/domain_build.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 008fc76..e036887 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>       acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
>                                  d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
>
> +    /* Map the EFI and ACPI tables to Dom0 */
> +    rc = map_regions_rw(d,
> +                        paddr_to_pfn(d->arch.efi_acpi_gpa),
> +                        PFN_UP(d->arch.efi_acpi_len),
> +                        paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));

The ACPI/EFI tables could potentially have data in the cache but are not 
written into the memory (because Xen is mapping the RAM with caching 
enabled). However, DOM0 may decide to map it with cache disabled. 
Therefore it would be possible for the domain to see wrong data.

So I think you need to clean the cache for this region.

> +    if ( rc != 0 )
> +    {
> +        printk(XENLOG_ERR "Unable to map 0x%"PRIx64

"Unable to map EFI/ACPI table ..." to differentiate with a similar error 
message in map_range_to_domain

> +               " - 0x%"PRIx64" in domain %d\n",
> +               d->arch.efi_acpi_gpa & PAGE_MASK,
> +               PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
> +               d->domain_id);
> +        return rc;
> +    }
> +
>       return 0;
>   }
>   #else
>

Regards,
Shannon Zhao March 22, 2016, 1:18 p.m. UTC | #2 
On 2016年03月22日 08:42, Julien Grall wrote:
> Hi Shannon,
> 
> On 17/03/2016 09:40, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> ---
>>   xen/arch/arm/domain_build.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 008fc76..e036887 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d,
>> struct kernel_info *kinfo)
>>       acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa,
>> d->arch.efi_acpi_len,
>>                                  d->arch.efi_acpi_table, &kinfo->mem,
>> tbl_add);
>>
>> +    /* Map the EFI and ACPI tables to Dom0 */
>> +    rc = map_regions_rw(d,
>> +                        paddr_to_pfn(d->arch.efi_acpi_gpa),
>> +                        PFN_UP(d->arch.efi_acpi_len),
>> +                       
>> paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
> 
> The ACPI/EFI tables could potentially have data in the cache but are not
> written into the memory (because Xen is mapping the RAM with caching
> enabled). However, DOM0 may decide to map it with cache disabled.
> Therefore it would be possible for the domain to see wrong data.
> 
> So I think you need to clean the cache for this region.
Oh, that would be good. Is there any existing function I can use?

Thanks,
Julien Grall March 22, 2016, 4:16 p.m. UTC | #3 
Hi Shannon,

On 22/03/16 13:18, Shannon Zhao wrote:
> On 2016年03月22日 08:42, Julien Grall wrote:
>> Hi Shannon,
>>
>> On 17/03/2016 09:40, Shannon Zhao wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.
>>>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> ---
>>>    xen/arch/arm/domain_build.c | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index 008fc76..e036887 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d,
>>> struct kernel_info *kinfo)
>>>        acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa,
>>> d->arch.efi_acpi_len,
>>>                                   d->arch.efi_acpi_table, &kinfo->mem,
>>> tbl_add);
>>>
>>> +    /* Map the EFI and ACPI tables to Dom0 */
>>> +    rc = map_regions_rw(d,
>>> +                        paddr_to_pfn(d->arch.efi_acpi_gpa),
>>> +                        PFN_UP(d->arch.efi_acpi_len),
>>> +
>>> paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
>>
>> The ACPI/EFI tables could potentially have data in the cache but are not
>> written into the memory (because Xen is mapping the RAM with caching
>> enabled). However, DOM0 may decide to map it with cache disabled.
>> Therefore it would be possible for the domain to see wrong data.
>>
>> So I think you need to clean the cache for this region.
> Oh, that would be good. Is there any existing function I can use?

You could reuse p2m_cache_flush. However this function will only flush 
cache for p2m_ram_* entries.

I think the best way would be to extend the CACHEFLUSH operations and 
maybe p2m_cache_flush (?).

Regards,
Shannon Zhao March 24, 2016, 2:59 p.m. UTC | #4 
On 2016年03月23日 00:16, Julien Grall wrote:
> Hi Shannon,
> 
> On 22/03/16 13:18, Shannon Zhao wrote:
>> On 2016年03月22日 08:42, Julien Grall wrote:
>>> Hi Shannon,
>>>
>>> On 17/03/2016 09:40, Shannon Zhao wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.
>>>>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>> ---
>>>>    xen/arch/arm/domain_build.c | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>> index 008fc76..e036887 100644
>>>> --- a/xen/arch/arm/domain_build.c
>>>> +++ b/xen/arch/arm/domain_build.c
>>>> @@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d,
>>>> struct kernel_info *kinfo)
>>>>        acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa,
>>>> d->arch.efi_acpi_len,
>>>>                                   d->arch.efi_acpi_table, &kinfo->mem,
>>>> tbl_add);
>>>>
>>>> +    /* Map the EFI and ACPI tables to Dom0 */
>>>> +    rc = map_regions_rw(d,
>>>> +                        paddr_to_pfn(d->arch.efi_acpi_gpa),
>>>> +                        PFN_UP(d->arch.efi_acpi_len),
>>>> +
>>>> paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
>>>
>>> The ACPI/EFI tables could potentially have data in the cache but are not
>>> written into the memory (because Xen is mapping the RAM with caching
>>> enabled). However, DOM0 may decide to map it with cache disabled.
>>> Therefore it would be possible for the domain to see wrong data.
>>>
>>> So I think you need to clean the cache for this region.
>> Oh, that would be good. Is there any existing function I can use?
> 
> You could reuse p2m_cache_flush. However this function will only flush
> cache for p2m_ram_* entries.
> 
> I think the best way would be to extend the CACHEFLUSH operations and
> maybe p2m_cache_flush (?).
So it needs to extend the CACHEFLUSH case to handle the p2m_mmio_direct
entry, right?

BTW, does the case you said exist? Before xen switches to Dom0, will it
invalid the cache?

Thanks,
Julien Grall March 24, 2016, 3:35 p.m. UTC | #5 
Hi Shannon,

On 24/03/16 14:59, Shannon Zhao wrote:
> On 2016年03月23日 00:16, Julien Grall wrote:
>> On 22/03/16 13:18, Shannon Zhao wrote:
>>> On 2016年03月22日 08:42, Julien Grall wrote:
>>>> On 17/03/2016 09:40, Shannon Zhao wrote:
>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>
>>>>> Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.
>>>>>
>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>>> ---
>>>>>     xen/arch/arm/domain_build.c | 15 +++++++++++++++
>>>>>     1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>>>> index 008fc76..e036887 100644
>>>>> --- a/xen/arch/arm/domain_build.c
>>>>> +++ b/xen/arch/arm/domain_build.c
>>>>> @@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d,
>>>>> struct kernel_info *kinfo)
>>>>>         acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa,
>>>>> d->arch.efi_acpi_len,
>>>>>                                    d->arch.efi_acpi_table, &kinfo->mem,
>>>>> tbl_add);
>>>>>
>>>>> +    /* Map the EFI and ACPI tables to Dom0 */
>>>>> +    rc = map_regions_rw(d,
>>>>> +                        paddr_to_pfn(d->arch.efi_acpi_gpa),
>>>>> +                        PFN_UP(d->arch.efi_acpi_len),
>>>>> +
>>>>> paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
>>>>
>>>> The ACPI/EFI tables could potentially have data in the cache but are not
>>>> written into the memory (because Xen is mapping the RAM with caching
>>>> enabled). However, DOM0 may decide to map it with cache disabled.
>>>> Therefore it would be possible for the domain to see wrong data.
>>>>
>>>> So I think you need to clean the cache for this region.
>>> Oh, that would be good. Is there any existing function I can use?
>>
>> You could reuse p2m_cache_flush. However this function will only flush
>> cache for p2m_ram_* entries.
>>
>> I think the best way would be to extend the CACHEFLUSH operations and
>> maybe p2m_cache_flush (?).
> So it needs to extend the CACHEFLUSH case to handle the p2m_mmio_direct
> entry, right?

Hmmm thinking a bit more about this solution. It would allow a domain to 
flush device memory region. And we don't want that.

There is actually a simpler solution, you can directly call 
clean_and_invalidate_dcache_va_range on your buffer. I.e

clean_and_invalidate_dcache_va_range(d->arch.efi_acpi_table, 
d->arch.efi_acpi_len);

>
> BTW, does the case you said exist? Before xen switches to Dom0, will it
> invalid the cache?

Yes, we had some issue in the past with the kernel, device tree, and 
initramfs passed to a domain. The domains boot with cache disabled, the 
domains will retrieve garbage if the data didn't reach the RAM.

Currently, Xen only flushes some part of the cache associated to the 
data copied in the guest memory (for instance see 
raw_copy_to_guest_flush_dcache).

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 008fc76..e036887 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1691,6 +1691,21 @@  static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
                                d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
 
+    /* Map the EFI and ACPI tables to Dom0 */
+    rc = map_regions_rw(d,
+                        paddr_to_pfn(d->arch.efi_acpi_gpa),
+                        PFN_UP(d->arch.efi_acpi_len),
+                        paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
+    if ( rc != 0 )
+    {
+        printk(XENLOG_ERR "Unable to map 0x%"PRIx64
+               " - 0x%"PRIx64" in domain %d\n",
+               d->arch.efi_acpi_gpa & PAGE_MASK,
+               PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
+               d->domain_id);
+        return rc;
+    }
+
     return 0;
 }
 #else