On 4/2/25 10:25, Andrea Righi via Devel wrote:
> Introduce the definition of a new acpi-generic-initiator object that can
> be used to link a PCI device with multiple NUMA nodes.
>
> Link: https://mail.gnu.org/archive/html/qemu-arm/2024-03/msg00358.html
>
> Signed-off-by: Andrea Righi <ari...@nvidia.com>
> ---
> src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++
> src/conf/domain_conf.h | 13 +++++++++++++
> src/conf/schemas/domaincommon.rng | 19 +++++++++++++++++++
> src/conf/virconftypes.h | 2 ++
> 4 files changed, 60 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c724638180..405f0207e9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3479,6 +3479,19 @@ virDomainHostdevDefNew(void)
> }
>
>
> +virDomainAcpiInitiatorDef *
> +virDomainAcpiInitiatorDefNew(void)
> +{
> + virDomainAcpiInitiatorDef *def;
> +
> + def = g_new0(virDomainAcpiInitiatorDef, 1);
> +
> + def->info = g_new0(virDomainDeviceInfo, 1);
> +
> + return def;
> +}
> +
> +
> static virDomainTPMDef *
> virDomainTPMDefNew(virDomainXMLOption *xmlopt)
> {
> @@ -3539,6 +3552,19 @@ void virDomainHostdevDefFree(virDomainHostdevDef *def)
> g_free(def);
> }
>
> +void
> +virDomainAcpiInitiatorDefFree(virDomainAcpiInitiatorDef *def)
> +{
> + if (!def)
> + return;
> +
> + g_free(def->name);
> + g_free(def->pciDev);
> +
> + virDomainDeviceInfoFree(def->info);
> + g_free(def);
> +}
> +
> void virDomainHubDefFree(virDomainHubDef *def)
> {
> if (!def)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 8dfadbb98d..9ee7ecfc4d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -122,6 +122,7 @@ struct _virDomainDeviceDef {
> virDomainAudioDef *audio;
> virDomainCryptoDef *crypto;
> virDomainPstoreDef *pstore;
> + virDomainAcpiInitiatorDef *acpiinitiator;
> } data;
> };
>
> @@ -353,6 +354,13 @@ typedef enum {
> VIR_DOMAIN_STARTUP_POLICY_LAST
> } virDomainStartupPolicy;
>
> +struct _virDomainAcpiInitiatorDef {
> + char *name;
> + char *pciDev;
> + int numaNode;
> + virDomainDeviceInfo *info; /* Guest address */
Firstly, this is always allocated (in virDomainAcpiInitiatorDefNew())
and thus doesn't need to be a pointer. Other devices live happily with
device info being a direct member of their structs.
Secondly, I'm not sure what "guest address" means, as this member is
never formatted onto the cmd line.
And finally, the virDomainDeviceInfo struct already has 'char *alias'
member which serves the same purpose as your 'char *name'. No need to
reinvent it. What we could do, is to have this struct look like this:
struct _virDomainAcpiInitiatorDef {
char *pciDev;
int numaNode;
virDomainDeviceInfo info;
};
And then:
1) virDomainAcpiInitiatorDefParseXML() would call
virDomainDeviceInfoParseXML() unconditionally,
2) virDomainAcpiInitiatorDefFormat would call virDomainDeviceInfoFormat(),
3) virDomainAcpiInitiatorDefValidate() would check that
acpiinitiator->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE,
Oh, and we do not really want users to provide object IDs that appear on
the cmd line (except for one specific case). Libvirt usually invents
device/object aliases on its own. You'll need to addjust
qemuAssignDeviceAliases() to generate aliases for this too.
Obviously, RNG shcema would need to be changed to follow this new approach.
Speaking of which, we generally try to avoid using dashes in element
name and prefer camelCase. Though, there is some prior art:
suspend-to-mem or suspend-to-disk or tb-cache.
> +};
> +
> /* basic device for direct passthrough */
> struct _virDomainHostdevDef {
> /* If 'parentnet' is non-NULL it means this host dev was
> @@ -3262,6 +3270,9 @@ struct _virDomainDef {
> size_t ntpms;
> virDomainTPMDef **tpms;
>
> + size_t nacpiinitiator;
> + virDomainAcpiInitiatorDef **acpiinitiator;
> +
> /* Only 1 */
> virDomainMemballoonDef *memballoon;
> virDomainNVRAMDef *nvram;
> @@ -3744,6 +3755,8 @@ virDomainVideoDef
> *virDomainVideoDefNew(virDomainXMLOption *xmlopt);
> void virDomainVideoDefFree(virDomainVideoDef *def);
> G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVideoDef, virDomainVideoDefFree);
> void virDomainVideoDefClear(virDomainVideoDef *def);
> +virDomainAcpiInitiatorDef *virDomainAcpiInitiatorDefNew(void);
> +void virDomainAcpiInitiatorDefFree(virDomainAcpiInitiatorDef *def);
Here, you probably want to declare a cleanup function too:
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainAcpiInitiatorDef,
virDomainAcpiInitiatorDefFree);
Michal
