RE: [PATCH v2 1/9] xen/x86: introduce a helper to update memory hotplug end

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 2022年7月13日 16:46
> To: Wei Chen <Wei.Chen@xxxxxxx>
> Cc: nd <nd@xxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; Roger Pau
> Monné <roger.pau@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 1/9] xen/x86: introduce a helper to update memory
> hotplug end
> 
> On 13.07.2022 10:22, Wei Chen wrote:
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: 2022年7月12日 19:54
> >>
> >> On 08.07.2022 16:54, Wei Chen wrote:
> >>> x86 provides a mem_hotplug variable to maintain the memory hotplug
> >>> end address. We want to move some codes from x86 to common, so that
> >>> it can be reused by other architectures. But not all architectures
> >>> have supported memory hotplug. So in this patch, we introduce this
> >>> helper to replace mem_hotplug direct access in these codes. This
> >>> will give the ability of stubbing this helper to those architectures
> >>> without memory hotplug support.
> >>
> >> What remains unclear to me is why Arm can't simply gain the necessary
> >> variable. Sooner or later you'll want to support hotplug there anyway,
> >
> > I am not sure my limited memory hotplug knowledge can answer your
> question
> > or not. As memory hotplug depends on hardware and firmware support. Now
> > for Arm, we only have ACPI firmware that can notify OS through GED event
> > (not very sure). But ACPI and device tree couldn't be enabled at the
> same
> > time from OS booting. In device tree based NUMA, we will enable device
> > tree, ACPI service will not be initialized, that means we can not get
> > notification of memory hotplug events from ACPI firmware.
> >
> > Of course, adding GED device nodes to the device tree, and getting these
> > events through GED interrupts should not be a big technical problem. But
> > there may be other reasons, and no one is planning to add memory hotplug
> > support to the device tree (perhaps need an ACPI-like specification or a
> > firmware abstraction interface). So if we want to implement Xen Arm
> memory
> > hotplug, it should also start from ACPI. So currently even if we gain
> the
> > variable in Arm, it will not be used. Device tree does not have property
> > to indicate a memory block can be hotplug or not.
> 
> But ACPI is possible to be enabled for Arm64, and hence hotplug could be
> made work that way. Therefore I view the variable as potentially useful
> on Arm as well, and hence introducing the variable might be less trouble
> than introducing the per-arch helper.
> 

Ok, I will try to expose mem_hotplug to common/mm.c. As both Arm and x86
code can access it, we can move related NUMA code to common Without this
helper. And mem_hotplug will always be zero for Arm until Arm support
memory hotplug. This should be harmless for Arm.

> >> I suppose. Mechanically the change is fine. Albeit maybe "top" instead
> >> of "boundary", and maybe also pass "node" even if x86 doesn't need it?
> >>
> >
> > Sorry, I am not very clear about these comments:
> > It makes sense to use mem_hotplug_update_top instead
> > of mem_hotplug_update_boundary. But how can I understand pass "node"?
> > did you mean mem_hotplug_update_top(node, end)? But mem_hotplug is
> > a global top for memory hotplug ranges. I don't think pass "node"
> > to this function can be useful.
> 
> Please separate "current implementation" from "abstract model". In the
> latter it would seem quite reasonable to me to have per-node upper
> bounds (or even per-node ranges). Therefore adding node (and, on top
> of what I did suggest before, region start) to the parameters of the
> new per-arch hook would seem a good move to me, even if at present
> only / at most the "end" parameter would actually be used.
> 

As we will export mem_hotplug to common, I think these changes are
not needed any more?

Cheers,
Wei Chen

> Jan