Re: [RFC PATCH V1 04/12] xen/arm: Introduce arch specific bits for IOREQ/DM features

From: Oleksandr <olekstysh@gmail.com>
To: Julien Grall <julien@xen.org>
Cc: paul@xen.org, xen-devel@lists.xenproject.org,
	'Stefano Stabellini' <sstabellini@kernel.org>,
	'Wei Liu' <wl@xen.org>,
	'Andrew Cooper' <andrew.cooper3@citrix.com>,
	'Ian Jackson' <ian.jackson@eu.citrix.com>,
	'George Dunlap' <george.dunlap@citrix.com>,
	'Oleksandr Tyshchenko' <oleksandr_tyshchenko@epam.com>,
	'Julien Grall' <julien.grall@arm.com>,
	'Jan Beulich' <jbeulich@suse.com>,
	'Daniel De Graaf' <dgdegra@tycho.nsa.gov>,
	'Volodymyr Babchuk' <Volodymyr_Babchuk@epam.com>
Subject: Re: [RFC PATCH V1 04/12] xen/arm: Introduce arch specific bits for IOREQ/DM features
Date: Mon, 17 Aug 2020 17:36:22 +0300	[thread overview]
Message-ID: <0003868f-c96c-8f31-d0ba-97cf9fd368ed@gmail.com> (raw)
In-Reply-To: <70241cb7-1b04-d3be-90d5-b075ee13ca8c@xen.org>

On 15.08.20 20:56, Julien Grall wrote:

Hi Julien.

> Hi,
>
> On 04/08/2020 15:01, Julien Grall wrote:
>> On 04/08/2020 08:49, Paul Durrant wrote:
>>>> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
>>>> index 931404c..b5fc066 100644
>>>> --- a/tools/libxc/xc_dom_arm.c
>>>> +++ b/tools/libxc/xc_dom_arm.c
>>>> @@ -26,11 +26,19 @@
>>>>   #include "xg_private.h"
>>>>   #include "xc_dom.h"
>>>>
>>>> -#define NR_MAGIC_PAGES 4
>>>> +
>>>>   #define CONSOLE_PFN_OFFSET 0
>>>>   #define XENSTORE_PFN_OFFSET 1
>>>>   #define MEMACCESS_PFN_OFFSET 2
>>>>   #define VUART_PFN_OFFSET 3
>>>> +#define IOREQ_SERVER_PFN_OFFSET 4
>>>> +
>>>> +#define NR_IOREQ_SERVER_PAGES 8
>>>> +#define NR_MAGIC_PAGES (4 + NR_IOREQ_SERVER_PAGES)
>>>> +
>>>> +#define GUEST_MAGIC_BASE_PFN (GUEST_MAGIC_BASE >> XC_PAGE_SHIFT)
>>>> +
>>>> +#define special_pfn(x)  (GUEST_MAGIC_BASE_PFN + (x))
>>>
>>> Why introduce 'magic pages' for Arm? It's quite a horrible hack that 
>>> we have begun to do away with by adding resource mapping.
>>
>> This would require us to mandate at least Linux 4.17 in a domain that 
>> will run an IOREQ server. If we don't mandate this, the minimum 
>> version would be 4.10 where DM OP was introduced.
>>
>> Because of XSA-300, we could technically not safely run an IOREQ 
>> server with existing Linux. So it is probably OK to enforce the use 
>> of the acquire interface.
> One more thing. We are using atomic operations on the IOREQ pages. As 
> our implementation is based on LL/SC instructions so far, we have 
> mitigation in place to prevent a domain DoS Xen. However, this relies 
> on the page to be mapped in a single domain at the time.
>
> AFAICT, with the legacy interface, the pages will be mapped in both 
> the target and the emulator. So this would defeat the mitigation we 
> have in place.
>
> Because the legacy interface is relying on foreign mapping, the page 
> has to be mapped in the target P2M. It might be possible to restrict 
> the access for the target by setting the p2m bits r, w to 0. This 
> would still allow the foreign mapping to work as we only check the p2m 
> type during mapping.
>
> Anyway, I think we agreed that we want to avoid to introduce the 
> legacy interface. But I wanted to answer just for completeness and 
> keep a record of potential pitfalls with the legacy interface on Arm.
ok, the HVMOP plumbing on Arm will be dropped for non-RFC series. It 
seems that xenforeignmemory_map_resource() does needed things. Of 
course, the corresponding Linux patch to support 
IOCTL_PRIVCMD_MMAP_RESOURCE was cherry-picked for that purpose (I am 
currently using v4.14).

Thank you.

-- 
Regards,

Oleksandr Tyshchenko