On 17.07.2023 17:17, Alejandro Vallejo wrote: > On Fri, Jul 14, 2023 at 12:36:11PM +0200, Jan Beulich wrote: >> On 14.07.2023 12:27, Alejandro Vallejo wrote: >>> On Thu, Jul 13, 2023 at 05:12:09PM +0200, Jan Beulich wrote: >>>> On 07.07.2023 18:07, Alejandro Vallejo wrote: >>>>> --- a/xen/include/xen/mm.h >>>>> +++ b/xen/include/xen/mm.h >>>>> @@ -31,6 +31,17 @@ >>>>> * (i.e. all devices assigned to) a guest share a single DMA address >>>>> space >>>>> * and, by default, Xen will ensure dfn == pfn. >>>>> * >>>>> + * pdx: Page InDeX >>>>> + * Indices into the frame table holding the per-page's book-keeping >>>>> + * metadata. A compression scheme may be used, so there's a possibly >>>>> non >>>>> + * identity mapping between valid(mfn) <-> valid(pdx). See the comments >>>>> + * in pdx.c for an in-depth explanation of that mapping. This also has >>>>> a >>>>> + * knock-on effect on the directmap, as "compressed" pfns have no >>>>> + * corresponding mapped frames. >>>> >>>> Didn't you mean to keep the directmap part optional, >>> I did. >>> >>>> which would call for saying "may" here (twice)? >>> That paragraph as-is doesn't really mandate a directmap. It merely state >>> that there are knock-on effects on directmap indexing, not that there's >>> always a directmap to index. >>> >>>> Yet then ... >>>> >>>> >>>>> --- a/xen/include/xen/pdx.h >>>>> +++ b/xen/include/xen/pdx.h >>>>> @@ -1,6 +1,73 @@ >>>>> #ifndef __XEN_PDX_H__ >>>>> #define __XEN_PDX_H__ >>>>> >>>>> +/* >>>>> + * PDX (Page inDeX) >>>>> + * >>>>> + * This file deals with optimisations pertaining to frame table and >>>>> + * directmap indexing, A pdx is an index into the frame table, which >>>>> + * typically also means an index into the directmap[1]. However, having >>>>> an >>>>> + * identity relationship between mfn and pdx could waste copious amounts >>>>> of >>>>> + * memory in empty frame table entries and page tables. There are some >>>>> + * techniques to bring memory wastage down. >>>>> + * >>>>> + * [1] Some ports apply further modifications to a pdx before indexing >>>>> the >>>>> + * directmap. This doesn't change the fact that the same compression >>>>> + * present in the frame table is also present in the directmap >>>>> + * whenever said map is present. >>>> >>>> .. you mention it here as non-optional as well. Didn't you tell me that >>>> Arm doesn't use compressed indexes into the directmap? >>>> >>>> Jan >>> >>> The [1] note states "whenever said map is present", meaning that it may not >>> be present. Saying it's optional is a stretch though. It's not like we can >>> choose right now. >>> >>>> Didn't you tell me that Arm doesn't use compressed indexes into the >>>> directmap? >>> arm32 doesn't have a directmap at all. arm64 uses biased pdx as indices >>> (they are offset by a constant), so they are still subject to compression. >> >> Hmm, then our understanding of "optional" was differing: I understood >> "use of compressed indexes is optional", when you apparently meant >> "the use of a directmap is optional". If this is the case, then I >> agree with the chosen wording. (Nevertheless using the suggested "may" >> wouldn't yield and incorrect outcome.) > > Like this? > > ``` > * pdx: Page InDeX > * Indices into the frame table holding the per-page's book-keeping > * metadata. A compression scheme may be used, so there's a possibly non > * identity mapping between valid(mfn) <-> valid(pdx). See the comments > * in pdx.c for an in-depth explanation of that mapping. This also may > * have a knock-on effect on the directmap, as "compressed" pfns may not > * have corresponding mapped frames. > ``` > > If so, sure. I don't mind either way. I'm happy for those 2 _may_s to be > added.
Yes please. Thanks, Jan
