Hi Stefano, > -----Original Message----- > From: Stefano Stabellini <sstabell...@kernel.org> > Subject: RE: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and > heap allocator > > On Tue, 30 Aug 2022, Henry Wang wrote: > > > -----Original Message----- > > > From: Michal Orzel <michal.or...@amd.com> > > > > > > > > Oh I think get your point. Let me try to explain myself and thanks for > your > > > > patience :)) > > > > > > > > The reserved heap region defined in the device tree should be used for > > > both > > > > Xenheap and domain heap, so if we reserved a too small region (<32M), > > > > an error should pop because the reserved region is not enough for > > > xenheap, > > > > and user should reserve more. > > > > [...] > > > > > > > >> But your check is against heap being to small (less than 32M). > > > >> So basically if the following check fails: > > > >> "( reserved_heap && reserved_heap_pages < 32<<(20- > PAGE_SHIFT) ) )" > > > >> it means that the heap region defined by a user is too small (not too > large), > > > >> because according to requirements it should be at least 32M. > > > > > > > > [...] > > > > So in that case, printing "Not enough space for xenheap" means the > > > reserved > > > > region cannot satisfy the minimal requirement of the space of xenheap > (at > > > least > > > > 32M), and this is in consistent with the check. > > > > > > Ok, it clearly depends on the way someone understands this sentence. > > > Currently this panic can be triggered if the heap size is too large and > > > should be read as "heap is too large to fit in because there is not enough > > > space > > > within RAM considering modules (e - s < size)". Usually (and also in this > case) > > > space refers to a region to contain another one. > > > > > > You are reusing the same message for different meaning, that is "user > > > defined too > > > small heap and this space (read as size) is not enough". > > > > Yes, thanks for the explanation. I think maybe rewording the message > > to "Not enough memory for allocating xenheap" would remove the > ambiguity > > to some extent? Because the user-defined heap region should cover both > > xenheap and domain heap at the same time, the small user-defined heap > > means "xenheap is too large to fit in the user-defined heap region", which > is > > in consistent with your interpretation of the current "xenheap is too large > to fit > > in because there is not enough space within RAM considering modules" > > I think we should have a separate check specific for the device tree > input parameters to make sure the region is correct, that way we can > have a specific error message, such as: > > "xen,static-heap address needs to be 32MB aligned and the size a > multiple of 32MB."
Sure, will follow this. Kind regards, Henry
