Re: git: e13c6a6fca71 - main - Teach the GICv3 driver about a vgic child

> On 21 Sep 2022, at 15:10, Jessica Clarke <jrtc27@freebsd.org> wrote:
> 
> On 21 Sept 2022, at 11:00, Andrew Turner <andrew@FreeBSD.org <mailto:andrew@FreeBSD.org>> wrote:
>> 
>> The branch main has been updated by andrew:
>> 
>> URL: https://cgit.FreeBSD.org/src/commit/?id=e13c6a6fca7177fb9143ce629c2e33b35b6c4219
>> 
>> commit e13c6a6fca7177fb9143ce629c2e33b35b6c4219
>> Author:     Andrew Turner <andrew@FreeBSD.org>
>> AuthorDate: 2022-09-14 16:29:29 +0000
>> Commit:     Andrew Turner <andrew@FreeBSD.org>
>> CommitDate: 2022-09-21 09:59:13 +0000
>> 
>>   Teach the GICv3 driver about a vgic child
>> 
>>   This will be used by bhyve to attach a virtual GIC driver.
>> 
>>   Sponsored by:   Innovate UK
>>   Sponsored by:   The FreeBSD Foundation
>>   Differential Revision: https://reviews.freebsd.org/D36590
>> ---
>> sys/arm/arm/gic_common.h      |  2 ++
>> sys/arm64/arm64/gic_v3.c      |  7 +++++++
>> sys/arm64/arm64/gic_v3_acpi.c | 36 ++++++++++++++++++++++++++++++++++--
>> sys/arm64/arm64/gic_v3_fdt.c  | 23 ++++++++++++++++++++++-
>> sys/arm64/arm64/gic_v3_var.h  |  1 +
>> 5 files changed, 66 insertions(+), 3 deletions(-)
>> 
>> diff --git a/sys/arm/arm/gic_common.h b/sys/arm/arm/gic_common.h
>> index 9e8fb19300ca..42ec44bb7fab 100644
>> --- a/sys/arm/arm/gic_common.h
>> +++ b/sys/arm/arm/gic_common.h
>> @@ -33,6 +33,7 @@
>> 
>> #define	GIC_IVAR_HW_REV		500
>> #define	GIC_IVAR_BUS		501
>> +#define	GIC_IVAR_VGIC		502
>> 
>> /* GIC_IVAR_BUS values */
>> #define	GIC_BUS_UNKNOWN		0
>> @@ -42,6 +43,7 @@
>> 
>> __BUS_ACCESSOR(gic, hw_rev, GIC, HW_REV, u_int);
>> __BUS_ACCESSOR(gic, bus, GIC, BUS, u_int);
>> +__BUS_ACCESSOR(gic, vgic, GIC, VGIC, u_int);
>> 
>> /* Software Generated Interrupts */
>> #define	GIC_FIRST_SGI		 0	/* Irqs 0-15 are SGIs/IPIs. */
>> diff --git a/sys/arm64/arm64/gic_v3.c b/sys/arm64/arm64/gic_v3.c
>> index 27f41c58fe92..b4678d95fee5 100644
>> --- a/sys/arm64/arm64/gic_v3.c
>> +++ b/sys/arm64/arm64/gic_v3.c
>> @@ -456,6 +456,7 @@ static int
>> gic_v3_read_ivar(device_t dev, device_t child, int which, uintptr_t *result)
>> {
>> 	struct gic_v3_softc *sc;
>> +	struct gic_v3_devinfo *di;
>> 
>> 	sc = device_get_softc(dev);
>> 
>> @@ -481,6 +482,12 @@ gic_v3_read_ivar(device_t dev, device_t child, int which, uintptr_t *result)
>> 		    ("gic_v3_read_ivar: Invalid bus type %u", sc->gic_bus));
>> 		*result = sc->gic_bus;
>> 		return (0);
>> +	case GIC_IVAR_VGIC:
>> +		di = device_get_ivars(child);
>> +		if (di == NULL)
>> +			return (EINVAL);
>> +		*result = di->is_vgic;
>> +		return (0);
>> 	}
>> 
>> 	return (ENOENT);
>> diff --git a/sys/arm64/arm64/gic_v3_acpi.c b/sys/arm64/arm64/gic_v3_acpi.c
>> index cbe4d5989ec2..cc24b7824ff1 100644
>> --- a/sys/arm64/arm64/gic_v3_acpi.c
>> +++ b/sys/arm64/arm64/gic_v3_acpi.c
>> @@ -48,6 +48,9 @@ __FBSDID("$FreeBSD$");
>> #include "gic_v3_reg.h"
>> #include "gic_v3_var.h"
>> 
>> +#define	GICV3_PRIV_VGIC		0x80000000
>> +#define	GICV3_PRIV_FLAGS	0x80000000
>> +
>> struct gic_v3_acpi_devinfo {
>> 	struct gic_v3_devinfo	di_gic_dinfo;
>> 	struct resource_list	di_rl;
>> @@ -86,6 +89,7 @@ struct madt_table_data {
>> 	ACPI_MADT_GENERIC_DISTRIBUTOR *dist;
>> 	int count;
>> 	bool rdist_use_gicc;
>> +	bool have_vgic;
>> };
>> 
>> static void
>> @@ -152,6 +156,8 @@ rdist_map(ACPI_SUBTABLE_HEADER *entry, void *arg)
>> 		BUS_SET_RESOURCE(madt_data->parent, madt_data->dev,
>> 		    SYS_RES_MEMORY, madt_data->count, intr->GicrBaseAddress,
>> 		    count);
>> +		if (intr->VgicInterrupt == 0)
>> +			madt_data->have_vgic = false;
>> 
>> 	default:
>> 		break;
>> @@ -164,6 +170,7 @@ gic_v3_acpi_identify(driver_t *driver, device_t parent)
>> 	struct madt_table_data madt_data;
>> 	ACPI_TABLE_MADT *madt;
>> 	vm_paddr_t physaddr;
>> +	uintptr_t private;
>> 	device_t dev;
>> 
>> 	physaddr = acpi_find_table(ACPI_SIG_MADT);
>> @@ -210,6 +217,7 @@ gic_v3_acpi_identify(driver_t *driver, device_t parent)
>> 
>> 	madt_data.dev = dev;
>> 	madt_data.rdist_use_gicc = false;
>> +	madt_data.have_vgic = true;
>> 	acpi_walk_subtables(madt + 1, (char *)madt + madt->Header.Length,
>> 	    rdist_map, &madt_data);
>> 	if (madt_data.count == 0) {
>> @@ -222,7 +230,12 @@ gic_v3_acpi_identify(driver_t *driver, device_t parent)
>> 		    rdist_map, &madt_data);
>> 	}
>> 
>> -	acpi_set_private(dev, (void *)(uintptr_t)madt_data.dist->Version);
>> +	private = madt_data.dist->Version;
>> +	/* Flag that the VGIC is in use */
>> +	if (madt_data.have_vgic)
>> +		private |= GICV3_PRIV_VGIC;
>> +
>> +	acpi_set_private(dev, (void *)private);
>> 
>> out:
>> 	acpi_unmap_table(madt);
>> @@ -232,7 +245,7 @@ static int
>> gic_v3_acpi_probe(device_t dev)
>> {
>> 
>> -	switch((uintptr_t)acpi_get_private(dev)) {
>> +	switch((uintptr_t)acpi_get_private(dev) & ~GICV3_PRIV_FLAGS) {
>> 	case ACPI_MADT_GIC_VERSION_V3:
>> 	case ACPI_MADT_GIC_VERSION_V4:
>> 		break;
>> @@ -390,9 +403,14 @@ gic_v3_add_children(ACPI_SUBTABLE_HEADER *entry, void *arg)
>> static void
>> gic_v3_acpi_bus_attach(device_t dev)
>> {
>> +	struct gic_v3_acpi_devinfo *di;
>> +	struct gic_v3_softc *sc;
>> 	ACPI_TABLE_MADT *madt;
>> +	device_t child;
>> 	vm_paddr_t physaddr;
>> 
>> +	sc = device_get_softc(dev);
>> +
>> 	physaddr = acpi_find_table(ACPI_SIG_MADT);
>> 	if (physaddr == 0)
>> 		return;
>> @@ -405,6 +423,20 @@ gic_v3_acpi_bus_attach(device_t dev)
>> 
>> 	acpi_walk_subtables(madt + 1, (char *)madt + madt->Header.Length,
>> 	    gic_v3_add_children, dev);
>> +	/* Add the vgic child if needed */
>> +	if (((uintptr_t)acpi_get_private(dev) & GICV3_PRIV_FLAGS) != 0) {
>> +		child = device_add_child(dev, "vgic", -1);
> 
> Do we have anything to attach to this, or is this just creating
> children that will never do anything?

This will be used by Bhyve to attach a virtual gic driver so we can use standard methods to handle interacting with the hardware.

Andrew