[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH V2] xen/gnttab: Store frame GFN in struct page_info on Arm

  • To: Oleksandr <olekstysh@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 16 Sep 2021 08:38:31 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=AJv9z/VlZXMdQt4wLVBVf1OgyUJJ2fFb2mp4WuvxNrc=; b=Ud0+EzfGLKLym4JkLqMJnQQKtUXniDtWBt81RQkg0yPDTfBa3orSpiEe23IF3Dkb1a0iBJwEGyWSXoZuNLe73qAYQghkJPAOIz761tgNocf3bCpMhNOMGGEcN85EsU5+b2Omsao5xAgcg21p5EkcFmnwCzjmx7iZEz2oKq/yitQtkzGBTtLpuslMaLlGWDFRmIdoqWumQMTx9Lwd0zjpAkAb335VhhfJlB8T550ktoJiQ/eEaMNKQOZr391PT+NQyoRTMy0LfX3m6uCYSvGjlww0Z+ZlnGwKLUUgx92xa1UOoX8fZDO6O9+QNyEw77/YwLovNudawoI977HcjYmidQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Gd0SIERTX2UNQfzcvykVr1NRNb3sO0+wAh2XqHW0MjZkiza3a6ebM/LPRaqnVlAhGevHpJHLGWBqLhdivWKgWI6ZGYoJSmTQ+YET5B8GnZC9/NpD5lrHmSrTomGr1qopu3KqIjhbiijo5wqR5H8OXSDBH0QpRzSufvfbtGRKjHBba72wvmNhwcC5wCZIS6rYoBID9boOikgLa6AconRYdFB6CYYrTF5hFP8WHq1JrEPqqsLFBRjCg5eRjuv/ea5ZvHrSoZHap1EIUDE3Y7f7JpP1h6L7Ra96XPuGQJsg/7PUuRLQR6LqKdIxTdC4YzfL5VWGYQx1vPaKRino6BMQ/g==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 16 Sep 2021 06:38:45 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 16.09.2021 00:13, Oleksandr wrote:
> On 15.09.21 13:06, Jan Beulich wrote:
>> On 14.09.2021 22:44, Oleksandr Tyshchenko wrote:
>>> --- a/xen/include/asm-arm/mm.h
>>> +++ b/xen/include/asm-arm/mm.h
>>> @@ -98,9 +98,18 @@ struct page_info
>>>   #define PGT_writable_page PG_mask(1, 1)  /* has writable mappings?        
>>>  */
>>>   #define PGT_type_mask     PG_mask(1, 1)  /* Bits 31 or 63.                
>>>  */
>>> - /* Count of uses of this frame as its current type. */
>>> -#define PGT_count_width   PG_shift(2)
>>> -#define PGT_count_mask    ((1UL<<PGT_count_width)-1)
>>> + /* 3-bit count of uses of this frame as its current type. */
>>> +#define PGT_count_base    PG_shift(4)
>>> +#define PGT_count_mask    PG_mask(7, 4)
>>> +
>>> +/*
>>> + * Stored in bits [27:0] or [59:0] GFN if page is used for grant table 
>>> frame.
>> I don't know enough Arm details to tell whether this is properly
>> one bit more than the maximum number of physical address bits.
>> Without the extra bit you wouldn't be able to tell apart a
>> guest specified GFN matching the value of PGT_INVALID_FRAME_GFN
>> from an entry which was set from INVALID_GFN.
> Really good point.
> 1. On Arm64 the p2m_ipa_bits could (theoretically) be 64-bit which, I 
> assume, corresponds to the maximum guest physical address (1 << 64) - 1 
> To store that GFN we need 52-bit. But, we provide 60-bit field which is 
> more than enough, I think. Practically, the maximum supported 
> p2m_ipa_bits is 48-bit, so the maximum supported GFN will occupy 36-bit 
> only. Everything is ok here.
> 2. On Arm32 the p2m_ipa_bits is 40-bit which, I assume, corresponds to 
> the maximum guest physical address (1 << 40) - 1 = 0xFFFFFFFFFF. To 
> store that GFN we need 28-bit. If I did the calculation correctly, what 
> we have on Arm32 is that PGT_INVALID_FRAME_GFN == maximum guest physical 
> address and it looks like we need and extra bit on Arm32. Do you think 
> we need to borrow one more bit from the count portion to stay on the 
> safe side?

I think so, unless there are further restrictions on the GFN range
that I'm unaware of.

For 64-bit, if you only need 52 bits, why do you make the field 60
bits wide? I'd recommend against "wasting" bits. Better keep the
count field as wide as possible.

>>> + * This only valid for the xenheap pages.
>>> + */
>>> +#define PGT_gfn_width     PG_shift(4)
>>> +#define PGT_gfn_mask      ((1UL<<PGT_gfn_width)-1)
>> Any reason you don't use PG_mask() here? Any open-coding is prone
>> to people later making mistakes.
> I failed to come up with idea how to do that without #ifdef. As GFN 
> starts at bit 0 different first parameter would be needed for PG_mask on 
> 32-bit and 64-bit systems.
> I wonder whether PGC_count_mask/PGT_count_mask are open-coded on Arm/x86 
> because of the same reason.

Hmm, that pre-existing open-coding isn't nice, but is perhaps a
good enough reason to keep what you have. (Personally I wouldn't
be afraid of adding an #ifdef here, but I realize views there may




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.