On 10/20/25 17:00, Peter Maydell wrote:
I don't really understand this code, I'm just looking at
it fresh, so my comment below might be wrong.
- /*
- * If guest pages remain on the first or last host pages,
- * adjust the deallocation to retain those guest pages.
- * The single page special case is required for the last page,
- * lest real_start overflow to zero.
- */
This comment says we need the special case for
"real_last - real_start < host_page_size" to avoid an overflow.
- if (real_last - real_start < host_page_size) {
- prot = 0;
We delete the special case...
- for (a = real_start; a < start; a += TARGET_PAGE_SIZE) {
- prot |= page_get_flags(a);
- }
- for (a = last; a < real_last; a += TARGET_PAGE_SIZE) {
- prot |= page_get_flags(a + 1);
- }
- if (prot != 0) {
- return 0;
- }
- } else {
- for (prot = 0, a = real_start; a < start; a += TARGET_PAGE_SIZE) {
- prot |= page_get_flags(a);
- }
- if (prot != 0) {
- real_start += host_page_size;
- }
+ /* end or real_end may have overflowed to 0, but that's okay. */
- for (prot = 0, a = last; a < real_last; a += TARGET_PAGE_SIZE) {
- prot |= page_get_flags(a + 1);
- }
- if (prot != 0) {
- real_last -= host_page_size;
- }
+ /* If [real_start,start) contains a mapped guest page, retain the first
page. */
+ for (prot = 0, off = 0; off < start - real_start; off += TARGET_PAGE_SIZE)
{
+ prot |= page_get_flags(real_start + off);
+ }
+ if (prot != 0) {
+ real_start += host_page_size;
...and now if real_start was the last page in the
address space, this addition will overflow it to zero.
g
Indeed, but the idea (which I didn't make clear enough, apologies) is
that this overflow is completely unproblematic. Provided you never
perform inequality comparisons on the value, overflow gives correct
results! Regardless though, Richard has asked that I revert to the old
strategy:
On 10/21/2025, Richard Henderson wrote:
No, it is not simpler with 'end', because end == 0 is 'valid' in the
sense that the range goes from [start, (abi_ptr)-1]. But having end
<= start is awkward in the extreme.
Thus we prefer the inclusive range [start, last] to the exclusive
range [start, end).
Not everything has been converted away from 'end', but we certainly
should not regress existing code.
r~
My perspective was that the only thing we actually lose from having 'end
<= start' is the ability to perform comparisons on these addresses, and
that this is better than the alternative, where we not only need the
single-page special case, but also the corrected 'real_last' computation
needs to use a temporarily-overflowed value:
real_last = ROUND_UP(last + 1, host_page_size) - 1;
I'm not here to argue style, though, so I'm happy to replace this diff
with a small one which only changes the 'real_last' definition. Will do
that in the next version of this series.
--
Matthew
