[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH][4.17] EFI: don't convert memory marked for runtime use to ordinary RAM
Hi Jan,
On 05/10/2022 12:55, Jan Beulich wrote:
On 05.10.2022 12:44, Julien Grall wrote:
On 04/10/2022 16:58, Jan Beulich wrote:
On 30.09.2022 14:51, Bertrand Marquis wrote:
On 30 Sep 2022, at 09:50, Jan Beulich <jbeulich@xxxxxxxx> wrote:
efi_init_memory() in both relevant places is treating EFI_MEMORY_RUNTIME
higher priority than the type of the range. To avoid accessing memory at
runtime which was re-used for other purposes, make
efi_arch_process_memory_map() follow suit. While on x86 in theory the
same would apply to EfiACPIReclaimMemory, we don't actually "reclaim"
E820_ACPI memory there and hence that type's handling can be left alone.
Fixes: bf6501a62e80 ("x86-64: EFI boot code")
Fixes: facac0af87ef ("x86-64: EFI runtime code")
Fixes: 6d70ea10d49f ("Add ARM EFI boot support")
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx> #arm
Thanks. However ...
---
Partly RFC for Arm, for two reasons:
On Arm I question the conversion of EfiACPIReclaimMemory, in two ways:
For one like on x86 such ranges would likely better be retained, as Dom0
may (will?) have a need to look at tables placed there. Plus converting
such ranges to RAM even if EFI_MEMORY_WB is not set looks suspicious to
me as well. I'd be inclined to make the latter adjustment right here
(while the other change probably would better be separate, if there
aren't actually reasons for the present behavior).
... any views on this WB aspect at least (also Stefano or Julien)? Would be
good to know before I send v2.
I don't quite understand what you are questioning here. Looking at the
code, EfiACPIReclaimMemory will not be converted to RAM but added in a
separate array.
Furthermore, all the EfiACPIReclaimMemory regions will be passed to dom0
(see acpi_create_efi_mmap_table()).
So to me the code looks correct.
Oh, I've indeed not paid enough attention to the first argument passed
to meminfo_add_bank(). I'm sorry for the extra noise. However, the
question I wanted to have addressed before sending out v3 was that
regarding the present using of memory when EFI_MEMORY_WB is not set.
Is that correct for the EfiACPIReclaimMemory case, i.e. is the
consumer (Dom0) aware that there might be a restriction?
Looking at the code, we always set EFI_MEMORY_WB for the reclaimable
region and the stage-2 mapping will be cachable.
So it looks like there would be a mismatch if EFI_MEMORY_WB is not set.
However, given the region is reclaimable, shouldn't this imply that the
flag is always set?
And would
this memory then be guaranteed to never be freed into the general pool
of RAM pages?
The region is not treated as RAM by Xen and not owned by the dom0.
Therefore, it should not be possible to free the page because
get_page_from_gfn() would not be able to get a reference.
Cheers,
--
Julien Grall
|
