| Message ID | 20250320223002.2915728-18-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: start make hw/arm/ common | expand |
On 3/20/25 15:29, Pierrick Bouvier wrote: > We introduce later a mechanism to skip cpu definitions inclusion, so we > can detect it here, and call the correct runtime function instead. > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > include/exec/target_page.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/exec/target_page.h b/include/exec/target_page.h > index 8e89e5cbe6f..aeddb25c743 100644 > --- a/include/exec/target_page.h > +++ b/include/exec/target_page.h > @@ -40,6 +40,9 @@ extern const TargetPageBits target_page; > # define TARGET_PAGE_MASK ((TARGET_PAGE_TYPE)target_page.mask) > # endif > # define TARGET_PAGE_SIZE (-(int)TARGET_PAGE_MASK) > +# ifndef TARGET_PAGE_BITS_MIN > +# define TARGET_PAGE_BITS_MIN qemu_target_page_bits_min() > +# endif > #else > # define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS > # define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS) Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. r~
On 3/21/25 11:05, Richard Henderson wrote: > On 3/20/25 15:29, Pierrick Bouvier wrote: >> We introduce later a mechanism to skip cpu definitions inclusion, so we >> can detect it here, and call the correct runtime function instead. >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> include/exec/target_page.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/include/exec/target_page.h b/include/exec/target_page.h >> index 8e89e5cbe6f..aeddb25c743 100644 >> --- a/include/exec/target_page.h >> +++ b/include/exec/target_page.h >> @@ -40,6 +40,9 @@ extern const TargetPageBits target_page; >> # define TARGET_PAGE_MASK ((TARGET_PAGE_TYPE)target_page.mask) >> # endif >> # define TARGET_PAGE_SIZE (-(int)TARGET_PAGE_MASK) >> +# ifndef TARGET_PAGE_BITS_MIN >> +# define TARGET_PAGE_BITS_MIN qemu_target_page_bits_min() >> +# endif >> #else >> # define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS >> # define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS) > > Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but > today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. > When you mention this, do you mean "constant accross all architectures", or a global (const) variable vs having a function call? > > r~
On 3/21/25 11:09, Pierrick Bouvier wrote: >> Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but >> today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. >> > > When you mention this, do you mean "constant accross all architectures", or a global > (const) variable vs having a function call? The first -- constant across all architectures. r~
On 3/21/25 12:27, Richard Henderson wrote: > On 3/21/25 11:09, Pierrick Bouvier wrote: >>> Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but >>> today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. >>> >> >> When you mention this, do you mean "constant accross all architectures", or a global >> (const) variable vs having a function call? > The first -- constant across all architectures. > That's great. Does choosing the min(set_of(TARGET_PAGE_BITS_MIN)) is what we want there, or is the answer more subtle than that? I went through that question, and was not sure what should be the answer. > > r~
On 3/21/25 13:11, Pierrick Bouvier wrote: > On 3/21/25 12:27, Richard Henderson wrote: >> On 3/21/25 11:09, Pierrick Bouvier wrote: >>>> Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but >>>> today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. >>>> >>> >>> When you mention this, do you mean "constant accross all architectures", or a global >>> (const) variable vs having a function call? >> The first -- constant across all architectures. >> > > That's great. > Does choosing the min(set_of(TARGET_PAGE_BITS_MIN)) is what we want there, or is the > answer more subtle than that? It will be, yes. This isn't as hard as it seems, because there are exactly two targets with TARGET_PAGE_BITS < 12: arm and avr. Because we still support armv4, TARGET_PAGE_BITS_MIN must be <= 10. AVR currently has TARGET_PAGE_BITS == 8, which is a bit of a problem. My first task is to allow avr to choose TARGET_PAGE_BITS_MIN >= 10. Which will leave us with TARGET_PAGE_BITS_MIN == 10. r~
On 3/21/25 15:19, Richard Henderson wrote: > On 3/21/25 13:11, Pierrick Bouvier wrote: >> On 3/21/25 12:27, Richard Henderson wrote: >>> On 3/21/25 11:09, Pierrick Bouvier wrote: >>>>> Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but >>>>> today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. >>>>> >>>> >>>> When you mention this, do you mean "constant accross all architectures", or a global >>>> (const) variable vs having a function call? >>> The first -- constant across all architectures. >>> >> >> That's great. >> Does choosing the min(set_of(TARGET_PAGE_BITS_MIN)) is what we want there, or is the >> answer more subtle than that? > > It will be, yes. > > This isn't as hard as it seems, because there are exactly two targets with > TARGET_PAGE_BITS < 12: arm and avr. > > Because we still support armv4, TARGET_PAGE_BITS_MIN must be <= 10. > > AVR currently has TARGET_PAGE_BITS == 8, which is a bit of a problem. > My first task is to allow avr to choose TARGET_PAGE_BITS_MIN >= 10. > > Which will leave us with TARGET_PAGE_BITS_MIN == 10. > Ok. From what I understand, we make sure tlb flags are stored in an immutable position, within virtual addresses related to guest, by using lower bits belonging to address range inside a given page, since page addresses are aligned on page size, leaving those bits free. bits [0..2) are bswap, watchpoint and check_aligned. bits [TARGET_PAGE_BITS_MIN - 5..TARGET_PAGE_BITS_MIN) are slow, discard_write, mmio, notdirty, and invalid mask. And the compile time check we have is to make sure we don't overlap those sets (would happen in TARGET_PAGE_BITS_MIN <= 7). I wonder why we can't use bits [3..8) everywhere, like it's done for AVR, even for bigger page sizes. I noticed the comment about "address alignment bits", but I'm confused why bits [0..2) can be used, and not upper ones. Are we storing something else in the middle on other archs, or did I miss some piece of the puzzle? Thanks, Pierrick > > r~
On 3/21/25 17:01, Pierrick Bouvier wrote: > On 3/21/25 15:19, Richard Henderson wrote: >> On 3/21/25 13:11, Pierrick Bouvier wrote: >>> On 3/21/25 12:27, Richard Henderson wrote: >>>> On 3/21/25 11:09, Pierrick Bouvier wrote: >>>>>> Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but >>>>>> today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. >>>>>> >>>>> >>>>> When you mention this, do you mean "constant accross all architectures", or a global >>>>> (const) variable vs having a function call? >>>> The first -- constant across all architectures. >>>> >>> >>> That's great. >>> Does choosing the min(set_of(TARGET_PAGE_BITS_MIN)) is what we want there, or is the >>> answer more subtle than that? >> >> It will be, yes. >> >> This isn't as hard as it seems, because there are exactly two targets with >> TARGET_PAGE_BITS < 12: arm and avr. >> >> Because we still support armv4, TARGET_PAGE_BITS_MIN must be <= 10. >> >> AVR currently has TARGET_PAGE_BITS == 8, which is a bit of a problem. >> My first task is to allow avr to choose TARGET_PAGE_BITS_MIN >= 10. >> >> Which will leave us with TARGET_PAGE_BITS_MIN == 10. >> > > Ok. > > From what I understand, we make sure tlb flags are stored in an > immutable position, within virtual addresses related to guest, by using > lower bits belonging to address range inside a given page, since page > addresses are aligned on page size, leaving those bits free. > > bits [0..2) are bswap, watchpoint and check_aligned. > bits [TARGET_PAGE_BITS_MIN - 5..TARGET_PAGE_BITS_MIN) are slow, > discard_write, mmio, notdirty, and invalid mask. > And the compile time check we have is to make sure we don't overlap > those sets (would happen in TARGET_PAGE_BITS_MIN <= 7). > > I wonder why we can't use bits [3..8) everywhere, like it's done for > AVR, even for bigger page sizes. I noticed the comment about "address > alignment bits", but I'm confused why bits [0..2) can be used, and not > upper ones. > > Are we storing something else in the middle on other archs, or did I > miss some piece of the puzzle? > After looking better, TLB_SLOW_FLAGS are not part of address, so we don't use bits [0..2). For a given TARGET_PAGE_SIZE, how do we define alignment bits? > Thanks, > Pierrick > >> >> r~ >
On 3/21/25 17:20, Pierrick Bouvier wrote: > On 3/21/25 17:01, Pierrick Bouvier wrote: >> On 3/21/25 15:19, Richard Henderson wrote: >>> On 3/21/25 13:11, Pierrick Bouvier wrote: >>>> On 3/21/25 12:27, Richard Henderson wrote: >>>>> On 3/21/25 11:09, Pierrick Bouvier wrote: >>>>>>> Mmm, ok I guess. Yesterday I would have suggested merging this with page-vary.h, but >>>>>>> today I'm actively working on making TARGET_PAGE_BITS_MIN a global constant. >>>>>>> >>>>>> >>>>>> When you mention this, do you mean "constant accross all architectures", or a global >>>>>> (const) variable vs having a function call? >>>>> The first -- constant across all architectures. >>>>> >>>> >>>> That's great. >>>> Does choosing the min(set_of(TARGET_PAGE_BITS_MIN)) is what we want there, or is the >>>> answer more subtle than that? >>> >>> It will be, yes. >>> >>> This isn't as hard as it seems, because there are exactly two targets with >>> TARGET_PAGE_BITS < 12: arm and avr. >>> >>> Because we still support armv4, TARGET_PAGE_BITS_MIN must be <= 10. >>> >>> AVR currently has TARGET_PAGE_BITS == 8, which is a bit of a problem. >>> My first task is to allow avr to choose TARGET_PAGE_BITS_MIN >= 10. >>> >>> Which will leave us with TARGET_PAGE_BITS_MIN == 10. >>> >> >> Ok. >> >> From what I understand, we make sure tlb flags are stored in an >> immutable position, within virtual addresses related to guest, by using >> lower bits belonging to address range inside a given page, since page >> addresses are aligned on page size, leaving those bits free. >> >> bits [0..2) are bswap, watchpoint and check_aligned. >> bits [TARGET_PAGE_BITS_MIN - 5..TARGET_PAGE_BITS_MIN) are slow, >> discard_write, mmio, notdirty, and invalid mask. >> And the compile time check we have is to make sure we don't overlap >> those sets (would happen in TARGET_PAGE_BITS_MIN <= 7). >> >> I wonder why we can't use bits [3..8) everywhere, like it's done for >> AVR, even for bigger page sizes. I noticed the comment about "address >> alignment bits", but I'm confused why bits [0..2) can be used, and not >> upper ones. >> >> Are we storing something else in the middle on other archs, or did I >> miss some piece of the puzzle? >> > > After looking better, TLB_SLOW_FLAGS are not part of address, so we don't use bits [0..2). > > For a given TARGET_PAGE_SIZE, how do we define alignment bits? Alignment bits are the least significant bits that must be 0 in order to enforce a particular alignment. The specific alignment is requested via MO_ALIGN et al as part of the guest memory reference. I think the piece you're missing is the softmmu fast path test in the generated code. We begin by indexing the tlb to find an entry. At that index, the entry may or may not match because (1) we have never looked up the page so the entry is empty, (2) we have looked up a different page that aliases, or (3) the page is present and (3a) correct, or (3b) invalidated, or (3c) some other condition that forces the slow path. The target address and the comparator have several fields: page address [63 ... TARGET_PAGE_BITS] page flags [TARGET_PAGE_BITS - 1 ... TARGET_PAGE_BITS - 5] unused [TARGET_PAGE_BITS - 6 ... align_bits], or empty. alignment [align_bits - 1 ... 0], or empty In the comparator, the unused and alignment bits are always zero; the page flags may be non-zero in order to force the comparison to fail. In the target address, we mask the page flags and unused bits; if the alignment bits of the address are set, then the address is of course unaligned and so the comparison fails. In order for all this work, the alignment field cannot overlap the page flags. The maximum alignment currently used by any guest is 5 bits, for Arm Neon, which means the minimum value for TARGET_PAGE_BITS_MIN is 10. r~
diff --git a/include/exec/target_page.h b/include/exec/target_page.h index 8e89e5cbe6f..aeddb25c743 100644 --- a/include/exec/target_page.h +++ b/include/exec/target_page.h @@ -40,6 +40,9 @@ extern const TargetPageBits target_page; # define TARGET_PAGE_MASK ((TARGET_PAGE_TYPE)target_page.mask) # endif # define TARGET_PAGE_SIZE (-(int)TARGET_PAGE_MASK) +# ifndef TARGET_PAGE_BITS_MIN +# define TARGET_PAGE_BITS_MIN qemu_target_page_bits_min() +# endif #else # define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS # define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
We introduce later a mechanism to skip cpu definitions inclusion, so we can detect it here, and call the correct runtime function instead. Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- include/exec/target_page.h | 3 +++ 1 file changed, 3 insertions(+)