qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: Leonid Bloch <lb.workbox@gmail.com>
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
	Ani Sinha <anisinha@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Eric Blake <eblake@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Dmitry Fleytman <dmitry.fleytman@gmail.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH v3 2/4] hw/acpi: Introduce the QEMU Battery
Date: Thu, 4 Sep 2025 14:54:44 +0200	[thread overview]
Message-ID: <20250904145444.40964692@fedora> (raw)
In-Reply-To: <20250827220054.37268-3-lb.workbox@gmail.com>

On Thu, 28 Aug 2025 01:00:48 +0300
Leonid Bloch <lb.workbox@gmail.com> wrote:

> The battery device communicates battery state to the guest via ACPI.
> It supports two modes of operation:
> 
> 1. QMP control mode (default): Battery state is controlled programmatically
>    via QMP commands, making the device deterministic and migration-safe.
> 
> 2. Host mirroring mode (optional): The device reflects the host's battery
>    state from sysfs. Probing occurs on guest ACPI requests and at timed
>    intervals (default 2 seconds, configurable via 'probe_interval' property
>    in milliseconds). State changes trigger ACPI notifications to the guest.
> 
> Properties:
> - 'use-qmp': Enable QMP control mode (default: true)
> - 'enable-sysfs': Enable host battery mirroring (default: false)
> - 'probe_interval': Probe interval in ms for sysfs mode (default: 2000)
> - 'sysfs_path': Override default sysfs path /sys/class/power_supply/
> 
> The device implements the ACPI_DEV_AML_IF interface to generate its
> own AML code, placing the BAT0 device directly under \_SB scope as
> per ACPI specification.
> 
> QMP commands:
> - battery-set-state: Set battery state (present, charging, capacity, rate)
> - query-battery: Query current battery state
> 
> This allows testing without host battery access and provides a stable
> interface for virtualization management systems.
> 
> Signed-off-by: Leonid Bloch <lb.workbox@gmail.com>
> Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
[...]

> +
> +static void build_battery_aml(AcpiDevAmlIf *adev, Aml *scope)
> +{
> +    Aml *dev, *field, *method, *pkg;
> +    Aml *bat_state, *bat_rate, *bat_charge;
> +    Aml *sb_scope;
> +    BatteryState *s = BATTERY_DEVICE(adev);
> +
> +    bat_state  = aml_local(0);
> +    bat_rate   = aml_local(1);
> +    bat_charge = aml_local(2);
> +
> +    sb_scope = aml_scope("\\_SB");
> +    dev = aml_device("BAT0");
> +    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C0A")));
> +
> +    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
> +    aml_append(method, aml_return(aml_int(0x1F)));
> +    aml_append(dev, method);
> +
> +    aml_append(dev, aml_operation_region("DBST", AML_SYSTEM_IO,
> +                                         aml_int(s->ioport),
> +                                         BATTERY_LEN));
> +    field = aml_field("DBST", AML_DWORD_ACC, AML_NOLOCK, AML_PRESERVE);
> +    aml_append(field, aml_named_field("BSTA", 32));
> +    aml_append(field, aml_named_field("BRTE", 32));
> +    aml_append(field, aml_named_field("BCRG", 32));
> +    aml_append(dev, field);
> +
> +    method = aml_method("_BIF", 0, AML_NOTSERIALIZED);
> +    pkg = aml_package(13);
> +    /* Power Unit */
> +    aml_append(pkg, aml_int(0));             /* mW */
> +    /* Design Capacity */
> +    aml_append(pkg, aml_int(BATTERY_FULL_CAP));
> +    /* Last Full Charge Capacity */
> +    aml_append(pkg, aml_int(BATTERY_FULL_CAP));
> +    /* Battery Technology */
> +    aml_append(pkg, aml_int(1));             /* Secondary */
> +    /* Design Voltage */
> +    aml_append(pkg, aml_int(BATTERY_VAL_UNKNOWN));
> +    /* Design Capacity of Warning */
> +    aml_append(pkg, aml_int(BATTERY_CAPACITY_OF_WARNING));
> +    /* Design Capacity of Low */
> +    aml_append(pkg, aml_int(BATTERY_CAPACITY_OF_LOW));
> +    /* Battery Capacity Granularity 1 */
> +    aml_append(pkg, aml_int(BATTERY_CAPACITY_GRANULARITY));
> +    /* Battery Capacity Granularity 2 */
> +    aml_append(pkg, aml_int(BATTERY_CAPACITY_GRANULARITY));
> +    /* Model Number */
> +    aml_append(pkg, aml_string("QBAT001"));  /* Model Number */
> +    /* Serial Number */
> +    aml_append(pkg, aml_string("SN00000"));  /* Serial Number */
> +    /* Battery Type */
> +    aml_append(pkg, aml_string("Virtual"));  /* Battery Type */
> +    /* OEM Information */
> +    aml_append(pkg, aml_string("QEMU"));     /* OEM Information */
> +    aml_append(method, aml_return(pkg));
> +    aml_append(dev, method);
> +
> +    pkg = aml_package(4);
> +    /* Battery State */
> +    aml_append(pkg, aml_int(0));
> +    /* Battery Present Rate */
> +    aml_append(pkg, aml_int(BATTERY_VAL_UNKNOWN));
> +    /* Battery Remaining Capacity */
> +    aml_append(pkg, aml_int(BATTERY_VAL_UNKNOWN));
> +    /* Battery Present Voltage */
> +    aml_append(pkg, aml_int(BATTERY_VAL_UNKNOWN));
> +    aml_append(dev, aml_name_decl("DBPR", pkg));
> +
> +    method = aml_method("_BST", 0, AML_NOTSERIALIZED);
> +    aml_append(method, aml_store(aml_name("BSTA"), bat_state));
> +    aml_append(method, aml_store(aml_name("BRTE"), bat_rate));
> +    aml_append(method, aml_store(aml_name("BCRG"), bat_charge));
> +    aml_append(method, aml_store(bat_state,
> +                                 aml_index(aml_name("DBPR"), aml_int(0))));
> +    aml_append(method, aml_store(bat_rate,
> +                                 aml_index(aml_name("DBPR"), aml_int(1))));
> +    aml_append(method, aml_store(bat_charge,
> +                                 aml_index(aml_name("DBPR"), aml_int(2))));
> +    aml_append(method, aml_return(aml_name("DBPR")));
> +    aml_append(dev, method);
> +
> +    aml_append(sb_scope, dev);
> +    aml_append(scope, sb_scope);
> +
> +    /* Device Check */
> +    method = aml_method("\\_GPE._E07", 0, AML_NOTSERIALIZED);
> +    aml_append(method, aml_notify(aml_name("\\_SB.BAT0"), aml_int(0x01)));
> +    aml_append(scope, method);
> +
> +    /* Status Change */
> +    method = aml_method("\\_GPE._E08", 0, AML_NOTSERIALIZED);
> +    aml_append(method, aml_notify(aml_name("\\_SB.BAT0"), aml_int(0x80)));
> +    aml_append(scope, method);
> +
> +    /* Information Change */
> +    method = aml_method("\\_GPE._E09", 0, AML_NOTSERIALIZED);
> +    aml_append(method, aml_notify(aml_name("\\_SB.BAT0"), aml_int(0x81)));
> +    aml_append(scope, method);
> +}
> +
> +static void battery_class_init(ObjectClass *class, const void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(class);
> +    AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(class);
> +
> +    dc->realize = battery_realize;
> +    device_class_set_props(dc, battery_device_properties);
> +    dc->vmsd = &battery_vmstate;
> +    adevc->build_dev_aml = build_battery_aml;
> +}
> +
> +static uint64_t battery_ioport_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    BatteryState *s = opaque;
> +
> +    battery_get_dynamic_status(s);
> +
> +    switch (addr) {
> +    case bsta_addr0:
> +        return s->state.acc[0];
> +    case bsta_addr1:
> +        return s->state.acc[1];
> +    case bsta_addr2:
> +        return s->state.acc[2];
> +    case bsta_addr3:
> +        return s->state.acc[3];
> +    case brte_addr0:
> +        return s->rate.acc[0];
> +    case brte_addr1:
> +        return s->rate.acc[1];
> +    case brte_addr2:
> +        return s->rate.acc[2];
> +    case brte_addr3:
> +        return s->rate.acc[3];
> +    case bcrg_addr0:
> +        return s->charge.acc[0];
> +    case bcrg_addr1:
> +        return s->charge.acc[1];
> +    case bcrg_addr2:
> +        return s->charge.acc[2];
> +    case bcrg_addr3:
> +        return s->charge.acc[3];

why do you handle it as byte access when in AML fields are
declared as DWORD access?

> +    default:
> +        warn_report("Battery: guest read unknown value.");

abort here, but I doubt that you can get here at all
if memory region is correctly sized.

> +        trace_battery_ioport_read_unknown();
> +        return 0;
> +    }
> +}
> +
> +static const MemoryRegionOps battery_ops = {
> +    .read = battery_ioport_read,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
same question wrt access size.
Suggest making it explicitly 4 byte and little-endinan

> +    },
> +};
> +
> +static void battery_instance_init(Object *obj)
> +{
> +    BatteryState *s = BATTERY_DEVICE(obj);
> +
> +    memory_region_init_io(&s->io, obj, &battery_ops, s, "battery",
> +                          BATTERY_LEN);
> +}
> +
> +static const TypeInfo battery_info = {
> +    .name          = TYPE_BATTERY,
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof(BatteryState),
> +    .class_init    = battery_class_init,
> +    .instance_init = battery_instance_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_ACPI_DEV_AML_IF },
> +        { },
> +    },
> +};
> +
> +static BatteryState *find_battery_device(void)
> +{
> +    Object *o = object_resolve_path_type("", TYPE_BATTERY, NULL);

also use 3rd argument and error out in case it's true,
so user would know what went wrong.
 
> +    if (!o) {
> +        return NULL;
> +    }
> +    return BATTERY_DEVICE(o);
> +}
> +
> +void qmp_battery_set_state(BatteryInfo *state, Error **errp)
> +{
> +    BatteryState *s = find_battery_device();
> +
> +    if (!s) {
> +        error_setg(errp, "No battery device found");
> +        return;
> +    }
> +
> +    s->qmp_present = state->present;
> +    s->qmp_charging = state->charging;
> +    s->qmp_discharging = state->discharging;
> +    s->qmp_charge_percent = state->charge_percent;
> +
> +    if (state->has_rate) {
> +        s->qmp_rate = state->rate;
> +    }
> +
> +    Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL);
> +    if (obj) {
> +        acpi_send_event(DEVICE(obj), ACPI_BATTERY_CHANGE_STATUS);
> +    }
> +}
> +
> +BatteryInfo *qmp_query_battery(Error **errp)
> +{
> +    BatteryState *s = find_battery_device();
> +    BatteryInfo *ret;
> +
> +    if (!s) {
> +        error_setg(errp, "No battery device found");
> +        return NULL;
> +    }
> +
> +    ret = g_new0(BatteryInfo, 1);
> +
> +    if (s->use_qmp_control) {
> +        ret->present = s->qmp_present;
> +        ret->charging = s->qmp_charging;
> +        ret->discharging = s->qmp_discharging;
> +        ret->charge_percent = s->qmp_charge_percent;
> +        ret->has_rate = true;
> +        ret->rate = s->qmp_rate;
> +    } else {
> +        battery_get_dynamic_status(s);
> +        ret->present = true;
> +        ret->charging = !!(s->state.val & BATTERY_CHARGING);
> +        ret->discharging = !!(s->state.val & BATTERY_DISCHARGING);
> +        ret->charge_percent = (s->charge.val * 100) / BATTERY_FULL_CAP;
> +        ret->has_rate = true;
> +        ret->rate = s->rate.val;
> +    }
> +
> +    ret->has_remaining_capacity = false;
> +    ret->has_design_capacity = true;
> +    ret->design_capacity = BATTERY_FULL_CAP;
> +
> +    return ret;
> +}
> +
> +static void battery_register_types(void)
> +{
> +    type_register_static(&battery_info);
> +}
> +
> +type_init(battery_register_types)
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index 73f02b9691..10379a7b2c 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -31,6 +31,7 @@ acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
>  if have_tpm
>    acpi_ss.add(files('tpm.c'))
>  endif
> +acpi_ss.add(when: 'CONFIG_BATTERY', if_true: files('battery.c'))
>  system_ss.add(when: 'CONFIG_ACPI', if_false: files('acpi-stub.c', 'aml-build-stub.c', 'ghes-stub.c', 'acpi_interface.c'))
>  system_ss.add(when: 'CONFIG_ACPI_PCI_BRIDGE', if_false: files('pci-bridge-stub.c'))
>  system_ss.add_all(when: 'CONFIG_ACPI', if_true: acpi_ss)
> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> index edc93e703c..dd3e815482 100644
> --- a/hw/acpi/trace-events
> +++ b/hw/acpi/trace-events
> @@ -87,3 +87,8 @@ acpi_nvdimm_read_io_port(void) "Alert: we never read _DSM IO Port"
>  acpi_nvdimm_dsm_mem_addr(uint64_t dsm_mem_addr) "dsm memory address 0x%" PRIx64
>  acpi_nvdimm_dsm_info(uint32_t revision, uint32_t handle, uint32_t function) "Revision 0x%" PRIx32 " Handle 0x%" PRIx32 " Function 0x%" PRIx32
>  acpi_nvdimm_invalid_revision(uint32_t revision) "Revision 0x%" PRIx32 " is not supported, expect 0x1"
> +
> +# battery.c
> +battery_realize(void) "Battery device realize entry"
> +battery_get_dynamic_status(uint32_t state, uint32_t rate, uint32_t charge) "Battery read state: 0x%"PRIx32", rate: %"PRIu32", charge: %"PRIu32
> +battery_ioport_read_unknown(void) "Battery read unknown"
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 3a0e2b8ebb..2c878fd112 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -39,6 +39,7 @@ config PC
>      imply VIRTIO_VGA
>      imply NVDIMM
>      imply FDC_ISA
> +    imply BATTERY
>      select I8259
>      select I8254
>      select PCKBD
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 423c4959fe..790b16e582 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1248,6 +1248,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>  
>          aml_append(sb_scope, dev);
>      }
> +
>      aml_append(dsdt, sb_scope);
>  
>      if (pm->pcihp_bridge_en || pm->pcihp_root_en) {
> diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h
> index 68d9d15f50..3064ef6734 100644
> --- a/include/hw/acpi/acpi_dev_interface.h
> +++ b/include/hw/acpi/acpi_dev_interface.h
> @@ -13,6 +13,7 @@ typedef enum {
>      ACPI_NVDIMM_HOTPLUG_STATUS = 16,
>      ACPI_VMGENID_CHANGE_STATUS = 32,
>      ACPI_POWER_DOWN_STATUS = 64,
> +    ACPI_BATTERY_CHANGE_STATUS = 128,
>  } AcpiEventStatusBits;
>  
>  #define TYPE_ACPI_DEVICE_IF "acpi-device-interface"
> diff --git a/include/hw/acpi/battery.h b/include/hw/acpi/battery.h
> new file mode 100644
> index 0000000000..5c5e83abfa
> --- /dev/null
> +++ b/include/hw/acpi/battery.h
> @@ -0,0 +1,33 @@
> +/*
> + * QEMU emulated battery device.
> + *
> + * Copyright (c) 2019 Janus Technologies, Inc. (http://janustech.com)
> + *
> + * Authors:
> + *     Leonid Bloch <lb.workbox@gmail.com>
> + *     Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> + *     Dmitry Fleytman <dmitry.fleytman@gmail.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + */
> +
> +#ifndef HW_ACPI_BATTERY_H
> +#define HW_ACPI_BATTERY_H
> +
> +#define TYPE_BATTERY                  "battery"
> +#define BATTERY_IOPORT_PROP           "ioport"
> +#define BATTERY_PATH_PROP             "sysfs_path"
> +#define BATTERY_PROBE_STATE_INTERVAL  "probe_interval"
> +
> +#define BATTERY_FULL_CAP     10000  /* mWh */
> +
> +#define BATTERY_CAPACITY_OF_WARNING   (BATTERY_FULL_CAP /  10)  /* 10% */
> +#define BATTERY_CAPACITY_OF_LOW       (BATTERY_FULL_CAP /  25)  /* 4%  */
> +#define BATTERY_CAPACITY_GRANULARITY  (BATTERY_FULL_CAP / 100)  /* 1%  */
> +
> +#define BATTERY_VAL_UNKNOWN  0xFFFFFFFF
> +
> +#define BATTERY_LEN          0x0C
> +
> +#endif
> diff --git a/qapi/acpi.json b/qapi/acpi.json
> index 906b3687a5..d1ad663bfd 100644
> --- a/qapi/acpi.json
> +++ b/qapi/acpi.json
> @@ -142,3 +142,76 @@
>  ##
>  { 'event': 'ACPI_DEVICE_OST',
>       'data': { 'info': 'ACPIOSTInfo' } }
> +
> +##
> +# @BatteryInfo:
> +#
> +# Battery state information
> +#
> +# @present: whether the battery is present
> +#
> +# @charging: whether the battery is charging
> +#
> +# @discharging: whether the battery is discharging
> +#
> +# @charge-percent: battery charge percentage (0-100)
> +#
> +# @rate: charge/discharge rate in mW (optional)
> +#
> +# @remaining-capacity: remaining capacity in mWh (optional)
> +#
> +# @design-capacity: design capacity in mWh (optional)
> +#
> +# Since: 10.2
> +##
> +{ 'struct': 'BatteryInfo',
> +  'data': { 'present': 'bool',
> +            'charging': 'bool',
> +            'discharging': 'bool',
> +            'charge-percent': 'int',
> +            '*rate': 'int',
> +            '*remaining-capacity': 'int',
> +            '*design-capacity': 'int' } }
> +
> +##
> +# @battery-set-state:
> +#
> +# Set the state of the emulated battery device
> +#
> +# @state: new battery state
> +#
> +
> +#
> +# Since: 10.2
> +#
> +# .. qmp-example::
> +#
> +#     -> { "execute": "battery-set-state",
> +#          "arguments": { "state": { "present": true,
> +#                                    "charging": true,
> +#                                    "discharging": false,
> +#                                    "charge-percent": 85 } } }
> +#     <- { "return": {} }
> +##
> +{ 'command': 'battery-set-state',
> +  'data': { 'state': 'BatteryInfo' } }
> +
> +##
> +# @query-battery:
> +#
> +# Query the current state of the emulated battery device
> +#
> +# Returns: current battery state
> +#
> +# Since: 10.2
> +#
> +# .. qmp-example::
> +#
> +#     -> { "execute": "query-battery" }
> +#     <- { "return": { "present": true,
> +#                      "charging": true,
> +#                      "discharging": false,
> +#                      "charge-percent": 85 } }
> +##
> +{ 'command': 'query-battery',
> +  'returns': 'BatteryInfo' }



  reply	other threads:[~2025-09-04 12:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-27 22:00 [PATCH v3 0/4] Introduce a battery, AC adapter, and lid button Leonid Bloch
2025-08-27 22:00 ` [PATCH v3 1/4] hw/acpi: Support extended GPE handling for additional ACPI devices Leonid Bloch
2025-09-04 13:21   ` Igor Mammedov
2025-08-27 22:00 ` [PATCH v3 2/4] hw/acpi: Introduce the QEMU Battery Leonid Bloch
2025-09-04 12:54   ` Igor Mammedov [this message]
2025-10-23 19:37   ` Denis V. Lunev
2025-10-23 20:58   ` Eric Blake
2025-08-27 22:00 ` [PATCH v3 3/4] hw/acpi: Introduce the QEMU AC adapter Leonid Bloch
2025-10-23 21:05   ` Eric Blake
2025-08-27 22:00 ` [PATCH v3 4/4] hw/acpi: Introduce the QEMU lid button Leonid Bloch
2025-09-04 12:41 ` [PATCH v3 0/4] Introduce a battery, AC adapter, and " Igor Mammedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250904145444.40964692@fedora \
    --to=imammedo@redhat.com \
    --cc=anisinha@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dmitry.fleytman@gmail.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=lb.workbox@gmail.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).