On 06/03/2024 5:09 pm, Ross Lagerwall wrote: > On Tue, Mar 5, 2024 at 2:17 PM Jan Beulich <jbeul...@suse.com> wrote: >> On 05.03.2024 13:11, Andrew Cooper wrote: >>> --- a/xen/include/xen/virtual_region.h >>> +++ b/xen/include/xen/virtual_region.h >>> @@ -16,6 +16,9 @@ struct virtual_region >>> const void *text_start; /* .text virtual address start. >>> */ >>> const void *text_end; /* .text virtual address end. */ >>> >>> + const void *rodata_start; /* .rodata virtual address >>> start (optional). */ >>> + const void *rodata_end; /* .rodata virtual address end. >>> */ >>> + >>> /* If this is NULL the default lookup mechanism is used. */ >>> symbols_lookup_t *symbols_lookup; >> While perhaps the least bad one can do without quite a bit more churn, >> I'm still irritated by a virtual region (singular) suddenly covering >> two ranges of VA space. At the very least I think the description should >> say a word of justification in this regard. An alternative, after all, >> could have been for livepatch code to register separate regions for >> rodata (if present in a patch). >> >> A follow-on question then would be why ordinary data isn't reflected in >> a virtual region. Aiui that's just because livepatch presently has no >> need for it. >> >> Underlying question to both: Is the virtual region concept indeed meant >> to be fully tied to livepatch and its needs? >> > Virtual regions were introduced for live patching but I don't think it > is completely tied to live patching. It was introduced so that any code > can participate in symbol lookup, bug frame and exception table entry > search, rather than special casing "if livepatch" in many places. > > I agree that the virtual region concept is being abused here - it's just > being used as a convenient place to store rodata start/end and doesn't > really have much to do with the text start & end / bug frame / exception > table entry search that the virtual region is intended for. > > Maybe Andrew can explain why he used this approach?
I feel the simplicity and obviousness of patch 3 speaks for itself. How do you propose fixing it differently. ~Andrew
