|
From: | Richard Henderson |
Subject: | Re: [RFC PATCH v2 1/7] hw/misc: Add device to help managing aliased memory regions |
Date: | Wed, 21 Apr 2021 18:33:55 -0700 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 |
On 4/19/21 2:43 AM, Philippe Mathieu-Daudé wrote:
Not really RFC, simply that I'v to add the technical description, but I'd like to know if there could be a possibility to not accept this device (because I missed something) before keeping working on it. So far it is only used in hobbyist boards. Cc: Peter Xu<peterx@redhat.com> Cc: Paolo Bonzini<pbonzini@redhat.com> --- include/hw/misc/aliased_region.h | 87 ++++++++++++++++++++ hw/misc/aliased_region.c | 132 +++++++++++++++++++++++++++++++ MAINTAINERS | 6 ++ hw/misc/Kconfig | 3 + hw/misc/meson.build | 1 + 5 files changed, 229 insertions(+) create mode 100644 include/hw/misc/aliased_region.h create mode 100644 hw/misc/aliased_region.c
Looks reasonable to me.
+ subregion_bits = 64 - clz64(s->span_size - 1); + s->mem.count = s->region_size >> subregion_bits; + assert(s->mem.count > 1); + subregion_size = 1ULL << subregion_bits;
So... subregion_size = pow2floor(s->span_size)?Why use a floor-ish computation here and pow2ceil later in aliased_mr_realize? Why use either floor or ceil as opposed to asserting that the size is in fact a power of 2? Why must the region be a power of 2, as opposed to e.g. a multiple of page_size? Or just the most basic requirement that region_size be a multiple of span_size?
r~
[Prev in Thread] | Current Thread | [Next in Thread] |