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.) Jan
