Re: [RFC PATCH v3 3/5] xen: add option to disable legacy backends

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi David,

"Woodhouse, David" <dwmw@xxxxxxxxxxxx> writes:

> [[S/MIME Signed Part:Undecided]]
> On Fri, 2023-11-24 at 23:24 +0000, Volodymyr Babchuk wrote:
>> This patch makes legacy backends optional. As was discussed at [1]
>> this is a solution to a problem when we can't run QEMU as a device
>> model in a non-privileged domain. This is because legacy backends
>> assume that they are always running in domain with ID = 0. Actually,
>> this may prevent running QEMU in a privileged domain with ID not equal
>> to zero.
>> 
>> To be able to disable legacy backends we need to alter couple of
>> source files that unintentionally depend on them. For example
>> xen-all.c used xen_pv_printf to report errors, while not providing any
>> additional like xendev pointer. Also, we need to move xenstore
>> structure from xen-legacy-backend.c, because it is apparently used in
>> xen-all.c.
>> 
>> With this patch it is possible to provide
>> "--disable-xen-legacy-backends" configure option to get QEMU binary
>> that can run in a driver domain. With price of not be able to use
>> legacy backends of course.
>> 
>> [1]
>> https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05022.html
>> 
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
>> 
>> ---
>> 
>> I am not sure if I made correct changes to the build system, thus this
>> patch is tagged as RFC.
>
> Hm, I was imagining a new CONFIG_LEGACY_XEN_BACKENDS option which would
> look a lot like CONFIG_XEN_BUS (which would now be only for the new
> XenBus code).
>

It was my original intention too. But it appears that it is not possible
to add Kconfig value and then make it configurable via ./config
script. As I understood it can be set only via defconfig file.

> This looks weird to me:
>
>> --- a/hw/display/meson.build
>> +++ b/hw/display/meson.build
>> @@ -14,7 +14,9 @@ system_ss.add(when: 'CONFIG_PL110', if_true:
>> files('pl110.c'))
>>  system_ss.add(when: 'CONFIG_SII9022', if_true: files('sii9022.c'))
>>  system_ss.add(when: 'CONFIG_SSD0303', if_true: files('ssd0303.c'))
>>  system_ss.add(when: 'CONFIG_SSD0323', if_true: files('ssd0323.c'))
>> -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
>> +if have_xen_legacy_backends
>> +  system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
>> +endif
>> 
>>  system_ss.add(when: 'CONFIG_VGA_PCI', if_true: files('vga-pci.c'))
>>  system_ss.add(when: 'CONFIG_VGA_ISA', if_true: files('vga-isa.c'))
>
> I'd prefer to see just:
>
> -system_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xenfb.c'))
> +system_ss.add(when: 'CONFIG_XEN_LEGACY_BACKENDS', if_true: files('xenfb.c'))

I tried, but it does not work this way. I need to create Kconfig
variable to do this, but then other problems appear.

>
> Probably also better to split out the bits in accel/xen/xen-all.c and
> hw/xen/xen-legacy-backend.c to a separate preparatory commit.

Okay, will do.

-- 
WBR, Volodymyr