[PATCH v3 1/4] hw/isa: add function to check for existence of device by its type

Liav Albani posted 4 patches 3 years, 11 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <ani@anisinha.ca>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
There is a newer version of this series
[PATCH v3 1/4] hw/isa: add function to check for existence of device by its type
Posted by Liav Albani 3 years, 11 months ago
This function enumerates all attached ISA devices in the machine, and
tries to compare a given device type name to the enumerated devices.
For example, this can help other code to determine if a i8042 controller
exists in the machine.

Signed-off-by: Liav Albani <liavalb@gmail.com>
---
 hw/isa/isa-bus.c     | 23 +++++++++++++++++++++++
 include/hw/isa/isa.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 6c31398dda..663aa36d29 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -222,6 +222,29 @@ void isa_build_aml(ISABus *bus, Aml *scope)
     }
 }
 
+bool isa_check_device_existence(const char *typename)
+{
+    /*
+     * If there's no ISA bus, we know for sure that the checked ISA device type
+     * doesn't exist in the machine.
+     */
+    if (isabus == NULL) {
+        return false;
+    }
+
+    BusChild *kid;
+    ISADevice *dev;
+
+    QTAILQ_FOREACH(kid, &isabus->parent_obj.children, sibling) {
+        dev = ISA_DEVICE(kid->child);
+        const char *object_type = object_get_typename(OBJECT(dev));
+        if (object_type && strcmp(object_type, typename) == 0) {
+            return true;
+        }
+    }
+    return false;
+}
+
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     ISADevice *d = ISA_DEVICE(dev);
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index d4417b34b6..65f0c7e28c 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -99,6 +99,7 @@ IsaDma *isa_get_dma(ISABus *bus, int nchan);
 MemoryRegion *isa_address_space(ISADevice *dev);
 MemoryRegion *isa_address_space_io(ISADevice *dev);
 ISADevice *isa_new(const char *name);
+bool isa_check_device_existence(const char *typename);
 ISADevice *isa_try_new(const char *name);
 bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
 ISADevice *isa_create_simple(ISABus *bus, const char *name);
-- 
2.35.1


Re: [PATCH v3 1/4] hw/isa: add function to check for existence of device by its type
Posted by Ani Sinha 3 years, 11 months ago

On Sat, 26 Feb 2022, Liav Albani wrote:

> This function enumerates all attached ISA devices in the machine, and
> tries to compare a given device type name to the enumerated devices.
> For example, this can help other code to determine if a i8042 controller
> exists in the machine.
>
> Signed-off-by: Liav Albani <liavalb@gmail.com>
> ---
>  hw/isa/isa-bus.c     | 23 +++++++++++++++++++++++
>  include/hw/isa/isa.h |  1 +
>  2 files changed, 24 insertions(+)
>
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index 6c31398dda..663aa36d29 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -222,6 +222,29 @@ void isa_build_aml(ISABus *bus, Aml *scope)
>      }
>  }
>
> +bool isa_check_device_existence(const char *typename)
> +{
> +    /*
> +     * If there's no ISA bus, we know for sure that the checked ISA device type
> +     * doesn't exist in the machine.
> +     */
> +    if (isabus == NULL) {

nit: I would do if (!isabus) instead to keep uniformity with other parts
of the code.

> +        return false;
> +    }
> +
> +    BusChild *kid;
> +    ISADevice *dev;
> +
> +    QTAILQ_FOREACH(kid, &isabus->parent_obj.children, sibling) {
> +        dev = ISA_DEVICE(kid->child);
> +        const char *object_type = object_get_typename(OBJECT(dev));
> +        if (object_type && strcmp(object_type, typename) == 0) {

nit: I would do !strcmp() instead.

> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
>  static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>  {
>      ISADevice *d = ISA_DEVICE(dev);
> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> index d4417b34b6..65f0c7e28c 100644
> --- a/include/hw/isa/isa.h
> +++ b/include/hw/isa/isa.h
> @@ -99,6 +99,7 @@ IsaDma *isa_get_dma(ISABus *bus, int nchan);
>  MemoryRegion *isa_address_space(ISADevice *dev);
>  MemoryRegion *isa_address_space_io(ISADevice *dev);
>  ISADevice *isa_new(const char *name);
> +bool isa_check_device_existence(const char *typename);

Please provide documentation for this function in line with other
functions like isa_register_ioport() and isa_register_portio_list()  in
the same header.
Re: [PATCH v3 1/4] hw/isa: add function to check for existence of device by its type
Posted by Liav Albani 3 years, 11 months ago
On 2/27/22 09:27, Ani Sinha wrote:
>
> On Sat, 26 Feb 2022, Liav Albani wrote:
>
>> This function enumerates all attached ISA devices in the machine, and
>> tries to compare a given device type name to the enumerated devices.
>> For example, this can help other code to determine if a i8042 controller
>> exists in the machine.
>>
>> Signed-off-by: Liav Albani <liavalb@gmail.com>
>> ---
>>   hw/isa/isa-bus.c     | 23 +++++++++++++++++++++++
>>   include/hw/isa/isa.h |  1 +
>>   2 files changed, 24 insertions(+)
>>
>> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
>> index 6c31398dda..663aa36d29 100644
>> --- a/hw/isa/isa-bus.c
>> +++ b/hw/isa/isa-bus.c
>> @@ -222,6 +222,29 @@ void isa_build_aml(ISABus *bus, Aml *scope)
>>       }
>>   }
>>
>> +bool isa_check_device_existence(const char *typename)
>> +{
>> +    /*
>> +     * If there's no ISA bus, we know for sure that the checked ISA device type
>> +     * doesn't exist in the machine.
>> +     */
>> +    if (isabus == NULL) {
> nit: I would do if (!isabus) instead to keep uniformity with other parts
> of the code.
Hmm, OK, I'll change it because it seems really fine to do that this way 
too :)
>
>> +        return false;
>> +    }
>> +
>> +    BusChild *kid;
>> +    ISADevice *dev;
>> +
>> +    QTAILQ_FOREACH(kid, &isabus->parent_obj.children, sibling) {
>> +        dev = ISA_DEVICE(kid->child);
>> +        const char *object_type = object_get_typename(OBJECT(dev));
>> +        if (object_type && strcmp(object_type, typename) == 0) {
> nit: I would do !strcmp() instead.
>
Hmm, OK, I'll change it because it seems really fine to do that this way 
too :)
>> +            return true;
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>>   static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>>   {
>>       ISADevice *d = ISA_DEVICE(dev);
>> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
>> index d4417b34b6..65f0c7e28c 100644
>> --- a/include/hw/isa/isa.h
>> +++ b/include/hw/isa/isa.h
>> @@ -99,6 +99,7 @@ IsaDma *isa_get_dma(ISABus *bus, int nchan);
>>   MemoryRegion *isa_address_space(ISADevice *dev);
>>   MemoryRegion *isa_address_space_io(ISADevice *dev);
>>   ISADevice *isa_new(const char *name);
>> +bool isa_check_device_existence(const char *typename);
> Please provide documentation for this function in line with other
> functions like isa_register_ioport() and isa_register_portio_list()  in
> the same header.

Ah, I see what you mean - I'll write short descriptive documentation 
like what there's for other functions :)

Thanks for the suggestions!

Best regards,
Liav