| Message ID | f1b5ee8652a20b2043965a4de5c2c64f662724bb.1681918194.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | enable MMU for RISC-V | expand |
On 19.04.2023 17:42, Oleksii Kurochko wrote: > --- a/xen/arch/riscv/include/asm/config.h > +++ b/xen/arch/riscv/include/asm/config.h > @@ -4,6 +4,37 @@ > #include <xen/const.h> > #include <xen/page-size.h> > > +/* > + * RISC-V64 Layout: > + * > + * From the riscv-privileged doc: > + * When mapping between narrower and wider addresses, > + * RISC-V zero-extends a narrower physical address to a wider size. > + * The mapping between 64-bit virtual addresses and the 39-bit usable > + * address space of Sv39 is not based on zero-extension but instead > + * follows an entrenched convention that allows an OS to use one or > + * a few of the most-significant bits of a full-size (64-bit) virtual > + * address to quickly distinguish user and supervisor address regions. > + * > + * It means that: > + * top VA bits are simply ignored for the purpose of translating to PA. > + * > + * The similar is true for other Sv{32, 39, 48, 57}. Personally I find it odd that Sv32 is mentioned here (there's no truncation / aliasing there aiui), and that Sv39 is mentioned here again (when that's what the earlier paragraph talks about). > + * ============================================================================ > + * Start addr | End addr | Size | VM area description > + * ============================================================================ > + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen > + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT > + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap These are all L2 slot 511 aiui, which may be worth mentioning especially since the top bits don't match the top bits further down in the table (because of the aliasing). > + * .................. unused .................. This is covering slot 510, which again may be worth mentioning. > + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 slot: 200-509) > + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 slot: 196-197) 1Gb is, if I'm not mistaken, a single L2 slot. Also assuming a 32-byte struct page_info (I don't think you'll get away with less than that, when even Arm32 requires this much), there's a mismatch between direct map and frame table size: With 4k pages, the scaling factor would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here to cover for (slightly more than) the 331Gb of memory you mean to be able to map? > + * 0000003080000000 | 00000030c0000000 | 1 GB | VMAP (L2 slot: 194-195) > + * .................. unused .................. There are further unused regions between these three entries, while imo will want making explicit (for the avoidance of doubt, if nothing else). Jan
On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: > On 19.04.2023 17:42, Oleksii Kurochko wrote: > > --- a/xen/arch/riscv/include/asm/config.h > > +++ b/xen/arch/riscv/include/asm/config.h > > @@ -4,6 +4,37 @@ > > #include <xen/const.h> > > #include <xen/page-size.h> > > > > +/* > > + * RISC-V64 Layout: > > + * > > + * From the riscv-privileged doc: > > + * When mapping between narrower and wider addresses, > > + * RISC-V zero-extends a narrower physical address to a wider > > size. > > + * The mapping between 64-bit virtual addresses and the 39-bit > > usable > > + * address space of Sv39 is not based on zero-extension but > > instead > > + * follows an entrenched convention that allows an OS to use one > > or > > + * a few of the most-significant bits of a full-size (64-bit) > > virtual > > + * address to quickly distinguish user and supervisor address > > regions. > > + * > > + * It means that: > > + * top VA bits are simply ignored for the purpose of translating > > to PA. > > + * > > + * The similar is true for other Sv{32, 39, 48, 57}. > > Personally I find it odd that Sv32 is mentioned here (there's no > truncation / > aliasing there aiui), and that Sv39 is mentioned here again (when > that's what > the earlier paragraph talks about). It looks that I wrote bad explanation. I meant that all the mentioned above ( that usable address space isn't based on zero-extension ) works for Sv{39, 48, 57} except size of usable address space ( 39-bit, 48-bit and 57-bit usable address space of 64-bit virtual address space ). And I agree that this is not for Sv32 as it covers full 32-bit virtual address space > > > + * > > =================================================================== > > ========= > > + * Start addr | End addr | Size | VM area > > description > > + * > > =================================================================== > > ========= > > + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen > > + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT > > + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap > > These are all L2 slot 511 aiui, which may be worth mentioning > especially since > the top bits don't match the top bits further down in the table > (because of the > aliasing). Than I'll add one more column where I'll put slot number > > > + * .................. unused .................. > > This is covering slot 510, which again may be worth mentioning. > > > + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 > > slot: 200-509) > > + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 > > slot: 196-197) > > 1Gb is, if I'm not mistaken, a single L2 slot. Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't included. I'll update the table. > > Also assuming a 32-byte struct page_info (I don't think you'll get > away with > less than that, when even Arm32 requires this much), there's a > mismatch > between direct map and frame table size: With 4k pages, the scaling > factor > would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here > to > cover for (slightly more than) the 331Gb of memory you mean to be > able to > map? For RV64 page_info size will be 56-byte and 32-byte for RV32 but you are right it should 3 Gb in that case what will be enough ( taking into account both available sizes of page_info structure ). Thanks for the catch. > > > + * 0000003080000000 | 00000030c0000000 | 1 GB | VMAP (L2 slot: > > 194-195) > > + * .................. unused .................. > > There are further unused regions between these three entries, while > imo will > want making explicit (for the avoidance of doubt, if nothing else). Yeah, there are some. I'll add them explicitly. ~ Oleksii
On 21.04.2023 16:41, Oleksii wrote: > On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: >> On 19.04.2023 17:42, Oleksii Kurochko wrote: >>> + * >>> =================================================================== >>> ========= >>> + * Start addr | End addr | Size | VM area >>> description >>> + * >>> =================================================================== >>> ========= >>> + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen >>> + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT >>> + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap >> >> These are all L2 slot 511 aiui, which may be worth mentioning >> especially since >> the top bits don't match the top bits further down in the table >> (because of the >> aliasing). > > Than I'll add one more column where I'll put slot number > >> >>> + * .................. unused .................. >> >> This is covering slot 510, which again may be worth mentioning. >> >>> + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 >>> slot: 200-509) >>> + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 >>> slot: 196-197) >> >> 1Gb is, if I'm not mistaken, a single L2 slot. > Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't > included. I'll update the table. > >> >> Also assuming a 32-byte struct page_info (I don't think you'll get >> away with >> less than that, when even Arm32 requires this much), there's a >> mismatch >> between direct map and frame table size: With 4k pages, the scaling >> factor >> would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here >> to >> cover for (slightly more than) the 331Gb of memory you mean to be >> able to >> map? > For RV64 page_info size will be 56-byte and 32-byte for RV32 but you > are right it should 3 Gb in that case what will be enough ( taking into > account both available sizes of page_info structure ). Since you say "both": I don't expect you will want to use 3Gb for RV32 there? The table is, at least for now, for RV64 / Sv39 only anyway. Jan
On 21.04.2023 16:41, Oleksii wrote: > On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: >> On 19.04.2023 17:42, Oleksii Kurochko wrote: >>> + * >>> =================================================================== >>> ========= >>> + * Start addr | End addr | Size | VM area >>> description >>> + * >>> =================================================================== >>> ========= >>> + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen >>> + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT >>> + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap >> >> These are all L2 slot 511 aiui, which may be worth mentioning >> especially since >> the top bits don't match the top bits further down in the table >> (because of the >> aliasing). > > Than I'll add one more column where I'll put slot number > >> >>> + * .................. unused .................. >> >> This is covering slot 510, which again may be worth mentioning. >> >>> + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 >>> slot: 200-509) >>> + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 >>> slot: 196-197) >> >> 1Gb is, if I'm not mistaken, a single L2 slot. > Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't > included. I'll update the table. > >> >> Also assuming a 32-byte struct page_info (I don't think you'll get >> away with >> less than that, when even Arm32 requires this much), there's a >> mismatch >> between direct map and frame table size: With 4k pages, the scaling >> factor >> would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here >> to >> cover for (slightly more than) the 331Gb of memory you mean to be >> able to >> map? > For RV64 page_info size will be 56-byte and 32-byte for RV32 but you > are right it should 3 Gb in that case what will be enough ( taking into > account both available sizes of page_info structure ). As to the plan to it being 56 bytes (i.e. like on Arm): Arm forever has had a 64-bit padding field at the end. My best guess is that the field was introduced to have a 32-byte struct on Arm32. But then why artificially increase the struct from 48 to 56 bytes on Arm64? And hence why have the same oddity on RV64? Jan
Hi, On 24/04/2023 10:33, Jan Beulich wrote: > On 21.04.2023 16:41, Oleksii wrote: >> On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: >>> On 19.04.2023 17:42, Oleksii Kurochko wrote: >>>> + * >>>> =================================================================== >>>> ========= >>>> + * Start addr | End addr | Size | VM area >>>> description >>>> + * >>>> =================================================================== >>>> ========= >>>> + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen >>>> + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT >>>> + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap >>> >>> These are all L2 slot 511 aiui, which may be worth mentioning >>> especially since >>> the top bits don't match the top bits further down in the table >>> (because of the >>> aliasing). >> >> Than I'll add one more column where I'll put slot number >> >>> >>>> + * .................. unused .................. >>> >>> This is covering slot 510, which again may be worth mentioning. >>> >>>> + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 >>>> slot: 200-509) >>>> + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 >>>> slot: 196-197) >>> >>> 1Gb is, if I'm not mistaken, a single L2 slot. >> Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't >> included. I'll update the table. >> >>> >>> Also assuming a 32-byte struct page_info (I don't think you'll get >>> away with >>> less than that, when even Arm32 requires this much), there's a >>> mismatch >>> between direct map and frame table size: With 4k pages, the scaling >>> factor >>> would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here >>> to >>> cover for (slightly more than) the 331Gb of memory you mean to be >>> able to >>> map? >> For RV64 page_info size will be 56-byte and 32-byte for RV32 but you >> are right it should 3 Gb in that case what will be enough ( taking into >> account both available sizes of page_info structure ). > > As to the plan to it being 56 bytes (i.e. like on Arm): Arm forever has > had a 64-bit padding field at the end. My best guess is that the field > was introduced to have a 32-byte struct on Arm32. I can't exactly remember. But I would like to rework the struct page_info on Arm64 because... But then why > artificially increase the struct from 48 to 56 bytes on Arm64? And hence > why have the same oddity on RV64? ... with 56 bytes, some struct page_info may cross a cache boundary. For RISC-V, I would recommend to make sure the struct page_info will never cross a cache boundary. Cheers,
On 24.04.2023 12:22, Julien Grall wrote: > Hi, > > On 24/04/2023 10:33, Jan Beulich wrote: >> On 21.04.2023 16:41, Oleksii wrote: >>> On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: >>>> On 19.04.2023 17:42, Oleksii Kurochko wrote: >>>>> + * >>>>> =================================================================== >>>>> ========= >>>>> + * Start addr | End addr | Size | VM area >>>>> description >>>>> + * >>>>> =================================================================== >>>>> ========= >>>>> + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen >>>>> + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT >>>>> + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap >>>> >>>> These are all L2 slot 511 aiui, which may be worth mentioning >>>> especially since >>>> the top bits don't match the top bits further down in the table >>>> (because of the >>>> aliasing). >>> >>> Than I'll add one more column where I'll put slot number >>> >>>> >>>>> + * .................. unused .................. >>>> >>>> This is covering slot 510, which again may be worth mentioning. >>>> >>>>> + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 >>>>> slot: 200-509) >>>>> + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 >>>>> slot: 196-197) >>>> >>>> 1Gb is, if I'm not mistaken, a single L2 slot. >>> Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't >>> included. I'll update the table. >>> >>>> >>>> Also assuming a 32-byte struct page_info (I don't think you'll get >>>> away with >>>> less than that, when even Arm32 requires this much), there's a >>>> mismatch >>>> between direct map and frame table size: With 4k pages, the scaling >>>> factor >>>> would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here >>>> to >>>> cover for (slightly more than) the 331Gb of memory you mean to be >>>> able to >>>> map? >>> For RV64 page_info size will be 56-byte and 32-byte for RV32 but you >>> are right it should 3 Gb in that case what will be enough ( taking into >>> account both available sizes of page_info structure ). >> >> As to the plan to it being 56 bytes (i.e. like on Arm): Arm forever has >> had a 64-bit padding field at the end. My best guess is that the field >> was introduced to have a 32-byte struct on Arm32. > > I can't exactly remember. But I would like to rework the struct > page_info on Arm64 because... > > But then why >> artificially increase the struct from 48 to 56 bytes on Arm64? And hence >> why have the same oddity on RV64? > > > ... with 56 bytes, some struct page_info may cross a cache boundary. I guess that's going to be challenging, unless you mean to go further up to 64 bytes? > For > RISC-V, I would recommend to make sure the struct page_info will never > cross a cache boundary. Since going up to 64 bytes is wasteful, and going down to 32 bytes likely isn't going to be easy, sticking to 48 bytes for now would seem reasonable to me. Jan
Hi Jan, On 24/04/2023 11:45, Jan Beulich wrote: > On 24.04.2023 12:22, Julien Grall wrote: >> Hi, >> >> On 24/04/2023 10:33, Jan Beulich wrote: >>> On 21.04.2023 16:41, Oleksii wrote: >>>> On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: >>>>> On 19.04.2023 17:42, Oleksii Kurochko wrote: >>>>>> + * >>>>>> =================================================================== >>>>>> ========= >>>>>> + * Start addr | End addr | Size | VM area >>>>>> description >>>>>> + * >>>>>> =================================================================== >>>>>> ========= >>>>>> + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen >>>>>> + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT >>>>>> + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap >>>>> >>>>> These are all L2 slot 511 aiui, which may be worth mentioning >>>>> especially since >>>>> the top bits don't match the top bits further down in the table >>>>> (because of the >>>>> aliasing). >>>> >>>> Than I'll add one more column where I'll put slot number >>>> >>>>> >>>>>> + * .................. unused .................. >>>>> >>>>> This is covering slot 510, which again may be worth mentioning. >>>>> >>>>>> + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 >>>>>> slot: 200-509) >>>>>> + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 >>>>>> slot: 196-197) >>>>> >>>>> 1Gb is, if I'm not mistaken, a single L2 slot. >>>> Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't >>>> included. I'll update the table. >>>> >>>>> >>>>> Also assuming a 32-byte struct page_info (I don't think you'll get >>>>> away with >>>>> less than that, when even Arm32 requires this much), there's a >>>>> mismatch >>>>> between direct map and frame table size: With 4k pages, the scaling >>>>> factor >>>>> would be 128 if I'm not mistaken. So perhaps you really mean 3Gb here >>>>> to >>>>> cover for (slightly more than) the 331Gb of memory you mean to be >>>>> able to >>>>> map? >>>> For RV64 page_info size will be 56-byte and 32-byte for RV32 but you >>>> are right it should 3 Gb in that case what will be enough ( taking into >>>> account both available sizes of page_info structure ). >>> >>> As to the plan to it being 56 bytes (i.e. like on Arm): Arm forever has >>> had a 64-bit padding field at the end. My best guess is that the field >>> was introduced to have a 32-byte struct on Arm32. >> >> I can't exactly remember. But I would like to rework the struct >> page_info on Arm64 because... >> >> But then why >>> artificially increase the struct from 48 to 56 bytes on Arm64? And hence >>> why have the same oddity on RV64? >> >> >> ... with 56 bytes, some struct page_info may cross a cache boundary. > > I guess that's going to be challenging, unless you mean to go further up > to 64 bytes? Yes. > >> For >> RISC-V, I would recommend to make sure the struct page_info will never >> cross a cache boundary. > > Since going up to 64 bytes is wasteful, Well yes. But this is a trade-off between performance and memory usage. With the current situation, you may have to pull two cache lines for struct page_info. I suspect you might see some slow down when using the grant. But I don't have any concrete numbers. > and going down to 32 bytes likely > isn't going to be easy, sticking to 48 bytes for now would seem reasonable > to me. It may be more difficult to argue for an increase (if we notice any performance degradation) in the future because this would reduce the memory usable for every users. Anyway, I haven't fully explore the problem on Arm yet and it is possible we could deal with any performance degradation differently (e.g. re-order the field and/or slightly increasing/decreasing the size). I thought I would point it out just in case the RISC-V folks care about it. Cheers,
On Mon, 2023-04-24 at 11:25 +0200, Jan Beulich wrote: > On 21.04.2023 16:41, Oleksii wrote: > > On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: > > > On 19.04.2023 17:42, Oleksii Kurochko wrote: > > > > + * > > > > =============================================================== > > > > ==== > > > > ========= > > > > + * Start addr | End addr | Size | VM area > > > > description > > > > + * > > > > =============================================================== > > > > ==== > > > > ========= > > > > + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen > > > > + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT > > > > + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap > > > > > > These are all L2 slot 511 aiui, which may be worth mentioning > > > especially since > > > the top bits don't match the top bits further down in the table > > > (because of the > > > aliasing). > > > > Than I'll add one more column where I'll put slot number > > > > > > > > > + * .................. unused .................. > > > > > > This is covering slot 510, which again may be worth mentioning. > > > > > > > + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct > > > > map(L2 > > > > slot: 200-509) > > > > + * 0000003100000000 | 0000003140000000 | 1 GB | > > > > Frametable(L2 > > > > slot: 196-197) > > > > > > 1Gb is, if I'm not mistaken, a single L2 slot. > > Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't > > included. I'll update the table. > > > > > > > > Also assuming a 32-byte struct page_info (I don't think you'll > > > get > > > away with > > > less than that, when even Arm32 requires this much), there's a > > > mismatch > > > between direct map and frame table size: With 4k pages, the > > > scaling > > > factor > > > would be 128 if I'm not mistaken. So perhaps you really mean 3Gb > > > here > > > to > > > cover for (slightly more than) the 331Gb of memory you mean to be > > > able to > > > map? > > For RV64 page_info size will be 56-byte and 32-byte for RV32 but > > you > > are right it should 3 Gb in that case what will be enough ( taking > > into > > account both available sizes of page_info structure ). > > Since you say "both": I don't expect you will want to use 3Gb for > RV32 > there? The table is, at least for now, for RV64 / Sv39 only anyway. > You are right. There is no any sense to use 3gb for RV32. ~ Oleksii
On Mon, 2023-04-24 at 11:33 +0200, Jan Beulich wrote: > On 21.04.2023 16:41, Oleksii wrote: > > On Thu, 2023-04-20 at 14:58 +0200, Jan Beulich wrote: > > > On 19.04.2023 17:42, Oleksii Kurochko wrote: > > > > + * > > > > =============================================================== > > > > ==== > > > > ========= > > > > + * Start addr | End addr | Size | VM area > > > > description > > > > + * > > > > =============================================================== > > > > ==== > > > > ========= > > > > + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen > > > > + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT > > > > + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap > > > > > > These are all L2 slot 511 aiui, which may be worth mentioning > > > especially since > > > the top bits don't match the top bits further down in the table > > > (because of the > > > aliasing). > > > > Than I'll add one more column where I'll put slot number > > > > > > > > > + * .................. unused .................. > > > > > > This is covering slot 510, which again may be worth mentioning. > > > > > > > + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct > > > > map(L2 > > > > slot: 200-509) > > > > + * 0000003100000000 | 0000003140000000 | 1 GB | > > > > Frametable(L2 > > > > slot: 196-197) > > > > > > 1Gb is, if I'm not mistaken, a single L2 slot. > > Yeah, it can be misunderstood. I meant [196, 197), so 197 isn't > > included. I'll update the table. > > > > > > > > Also assuming a 32-byte struct page_info (I don't think you'll > > > get > > > away with > > > less than that, when even Arm32 requires this much), there's a > > > mismatch > > > between direct map and frame table size: With 4k pages, the > > > scaling > > > factor > > > would be 128 if I'm not mistaken. So perhaps you really mean 3Gb > > > here > > > to > > > cover for (slightly more than) the 331Gb of memory you mean to be > > > able to > > > map? > > For RV64 page_info size will be 56-byte and 32-byte for RV32 but > > you > > are right it should 3 Gb in that case what will be enough ( taking > > into > > account both available sizes of page_info structure ). > > As to the plan to it being 56 bytes (i.e. like on Arm): Arm forever > has > had a 64-bit padding field at the end. My best guess is that the > field > was introduced to have a 32-byte struct on Arm32. But then why > artificially increase the struct from 48 to 56 bytes on Arm64? And > hence > why have the same oddity on RV64? I didn't think about that. I just re-used page_info structure from ARM. I'll think about your comment. Thanks. ~ Oleksii
Hi Julien, On Mon, 2023-04-24 at 12:08 +0100, Julien Grall wrote: > > > > > > On 19.04.2023 17:42, Oleksii Kurochko wrote: > > > > > > > + * > > > > > > > ========================================================= > > > > > > > ========== > > > > > > > ========= > > > > > > > + * Start addr | End addr | Size | VM > > > > > > > area > > > > > > > description > > > > > > > + * > > > > > > > ========================================================= > > > > > > > ========== > > > > > > > ========= > > > > > > > + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen > > > > > > > + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT > > > > > > > + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | > > > > > > > Fixmap > > > > > > > > > > > > These are all L2 slot 511 aiui, which may be worth > > > > > > mentioning > > > > > > especially since > > > > > > the top bits don't match the top bits further down in the > > > > > > table > > > > > > (because of the > > > > > > aliasing). > > > > > > > > > > Than I'll add one more column where I'll put slot number > > > > > > > > > > > > > > > > > > + * .................. unused .................. > > > > > > > > > > > > This is covering slot 510, which again may be worth > > > > > > mentioning. > > > > > > > > > > > > > + * 0000003200000000 | 0000007f40000000 | 331 GB | > > > > > > > Direct map(L2 > > > > > > > slot: 200-509) > > > > > > > + * 0000003100000000 | 0000003140000000 | 1 GB | > > > > > > > Frametable(L2 > > > > > > > slot: 196-197) > > > > > > > > > > > > 1Gb is, if I'm not mistaken, a single L2 slot. > > > > > Yeah, it can be misunderstood. I meant [196, 197), so 197 > > > > > isn't > > > > > included. I'll update the table. > > > > > > > > > > > > > > > > > Also assuming a 32-byte struct page_info (I don't think > > > > > > you'll get > > > > > > away with > > > > > > less than that, when even Arm32 requires this much), > > > > > > there's a > > > > > > mismatch > > > > > > between direct map and frame table size: With 4k pages, the > > > > > > scaling > > > > > > factor > > > > > > would be 128 if I'm not mistaken. So perhaps you really > > > > > > mean 3Gb here > > > > > > to > > > > > > cover for (slightly more than) the 331Gb of memory you mean > > > > > > to be > > > > > > able to > > > > > > map? > > > > > For RV64 page_info size will be 56-byte and 32-byte for RV32 > > > > > but you > > > > > are right it should 3 Gb in that case what will be enough ( > > > > > taking into > > > > > account both available sizes of page_info structure ). > > > > > > > > As to the plan to it being 56 bytes (i.e. like on Arm): Arm > > > > forever has > > > > had a 64-bit padding field at the end. My best guess is that > > > > the field > > > > was introduced to have a 32-byte struct on Arm32. > > > > > > I can't exactly remember. But I would like to rework the struct > > > page_info on Arm64 because... > > > > > > But then why > > > > artificially increase the struct from 48 to 56 bytes on Arm64? > > > > And hence > > > > why have the same oddity on RV64? > > > > > > > > > ... with 56 bytes, some struct page_info may cross a cache > > > boundary. > > > > I guess that's going to be challenging, unless you mean to go > > further up > > to 64 bytes? > > Yes. > > > > > > For > > > RISC-V, I would recommend to make sure the struct page_info will > > > never > > > cross a cache boundary. Do you mean that size(struct page_info) <= cache line size? > > > > Since going up to 64 bytes is wasteful, > > Well yes. But this is a trade-off between performance and memory > usage. > With the current situation, you may have to pull two cache lines for > struct page_info. Just for my understanding. stuct page_info will consume two cache lines (it happens in case of 3 consecutive which exceed the size of the cache line) if they are not aligned on 64 bytes boundary as cache line size on ARM is 128 ( if believe to ARM's asm/cache.h ). Shouldn't alignment ( by adding empty fields inside page_info ) fix an issue? It looks like if the size of page_info will not be aligned that it will be always performance issue. > > I suspect you might see some slow down when using the grant. But I > don't > have any concrete numbers. > > > and going down to 32 bytes likely > > isn't going to be easy, sticking to 48 bytes for now would seem > > reasonable > > to me. > > It may be more difficult to argue for an increase (if we notice any > performance degradation) in the future because this would reduce the > memory usable for every users. > > Anyway, I haven't fully explore the problem on Arm yet and it is > possible we could deal with any performance degradation differently > (e.g. re-order the field and/or slightly increasing/decreasing the > size). > > I thought I would point it out just in case the RISC-V folks care > about it. I think it will be useful as RISC-V uses the same page_info structure. ~ Oleksii
On 29.04.2023 12:05, Oleksii wrote: >>>> For >>>> RISC-V, I would recommend to make sure the struct page_info will >>>> never >>>> cross a cache boundary. > Do you mean that size(struct page_info) <= cache line size? I don't think that's what was meant. Instead I expect the goal is for no struct page_info instance to ever cross a cache line boundary. IOW one of sizeof(struct page_info) % cachelinesize == 0 or cachelinesize % sizeof(struct page_info) == 0, or in yet different terms (with the expectation that cache lines are always a power of two in size) sizeof(struct page_info) == 2**n. Yet unless you're able to fit everything in 32 bytes, that'll mean more overhead than strictly necessary. Jan
diff --git a/xen/arch/riscv/include/asm/config.h b/xen/arch/riscv/include/asm/config.h index 763a922a04..0c860e88ce 100644 --- a/xen/arch/riscv/include/asm/config.h +++ b/xen/arch/riscv/include/asm/config.h @@ -4,6 +4,37 @@ #include <xen/const.h> #include <xen/page-size.h> +/* + * RISC-V64 Layout: + * + * From the riscv-privileged doc: + * When mapping between narrower and wider addresses, + * RISC-V zero-extends a narrower physical address to a wider size. + * The mapping between 64-bit virtual addresses and the 39-bit usable + * address space of Sv39 is not based on zero-extension but instead + * follows an entrenched convention that allows an OS to use one or + * a few of the most-significant bits of a full-size (64-bit) virtual + * address to quickly distinguish user and supervisor address regions. + * + * It means that: + * top VA bits are simply ignored for the purpose of translating to PA. + * + * The similar is true for other Sv{32, 39, 48, 57}. + * + * ============================================================================ + * Start addr | End addr | Size | VM area description + * ============================================================================ + * FFFFFFFFC0000000 | FFFFFFFFC0200000 | 2 MB | Xen + * FFFFFFFFC0200000 | FFFFFFFFC0600000 | 4 MB | FDT + * FFFFFFFFC0600000 | FFFFFFFFC0800000 | 2 MB | Fixmap + * .................. unused .................. + * 0000003200000000 | 0000007f40000000 | 331 GB | Direct map(L2 slot: 200-509) + * 0000003100000000 | 0000003140000000 | 1 GB | Frametable(L2 slot: 196-197) + * 0000003080000000 | 00000030c0000000 | 1 GB | VMAP (L2 slot: 194-195) + * .................. unused .................. + * ============================================================================ + */ + #if defined(CONFIG_RISCV_64) # define LONG_BYTEORDER 3 # define ELFSIZE 64
Also it was added explanation about ignoring of top VA bits Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V5: * the patch was introduced in the current patch series. --- xen/arch/riscv/include/asm/config.h | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)