[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V3 4/6] xen/unpopulated-alloc: Add mechanism to use Xen resource
On 08.12.21 01:36, Stefano Stabellini wrote: Hi Stefano On Thu, 25 Nov 2021, Oleksandr wrote:Please note the following: for V3 arch_xen_unpopulated_init() was moved to init() as was agreed and gained __init specifier. So the target_resource is initialized there. With current patch series applied if CONFIG_XEN_UNPOPULATED_ALLOC is enabled: 1. On Arm, under normal circumstances, the xen_alloc_unpopulated_pages() won't be called “before” arch_xen_unpopulated_init(). It will only be called "before" when either ACPI is in use or something wrong happened with DT (and we failed to read xen_grant_frames), so we fallback to xen_xlate_map_ballooned_pages() in arm/xen/enlighten.c:xen_guest_init(), please see "arm/xen: Switch to use gnttab_setup_auto_xlat_frames() for DT" for details. But in that case, I think, it doesn't matter much whether xen_alloc_unpopulated_pages() is called "before" of "after" target_resource initialization, as we don't have extended regions in place the target_resource will remain invalid even after initialization, so xen_alloc_ballooned_pages() will be used in both scenarios. 2. On x86, I am not quite sure which modes use unpopulated-alloc (PVH?), but it looks like xen_alloc_unpopulated_pages() can (and will) be called “before” arch_xen_unpopulated_init(). At least, I see that xen_xlate_map_ballooned_pages() is called in x86/xen/grant-table.c:xen_pvh_gnttab_setup(). According to the initcall levels for both xen_pvh_gnttab_setup() and init() I expect the former to be called earlier. If it is true, the sentence in the commit description which mentions that “behaviour on x86 is not changed” is not precise. I don’t think it would be correct to fallback to xen_alloc_ballooned_pages() just because we haven’t initialized target_resource yet (on x86 it is just assigning it iomem_resource), at least this doesn't look like an expected behaviour and unlikely would be welcome. I am wondering whether it would be better to move arch_xen_unpopulated_init() to a dedicated init() marked with an appropriate initcall level (early_initcall?) to make sure it will always be called *before* xen_xlate_map_ballooned_pages(). What do you think?... here (#2). Or I really missed something and there wouldn't be an issue?Yes, I see your point. Yeah, it makes sense to make sure that drivers/xen/unpopulated-alloc.c:init is executed before xen_pvh_gnttab_setup. If we move it to early_initcall, then we end up running it before xen_guest_init on ARM. But that might be fine: it looks like it should work OK and would also allow us to execute xen_xlate_map_ballooned_pages with target_resource already set. So I'd say go for it :) Thank you for the confirmation! In order to be on the safe side, I would probably leave drivers/xen/unpopulated-alloc.c:init as is, I mean with current subsys initcall level (it expects the extra memory regions to be already filled) and create a separate unpopulated_init() to put arch_xen_unpopulated_init() into. Something like the following: static int __init unpopulated_init(void) { int ret; if (!xen_domain()) return -ENODEV; ret = arch_xen_unpopulated_init(&target_resource); if (ret) { pr_err("xen:unpopulated: Cannot initialize target resource\n"); target_resource = NULL; } return ret; } early_initcall(unpopulated_init); -- Regards, Oleksandr Tyshchenko
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |