On Fri, Feb 07, 2014 at 01:51:33PM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> ---
> hw/i386/acpi-build.c | 62
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 62 insertions(+), 0 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f0bedbd..ce5f715 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -230,8 +230,13 @@ static void acpi_get_pci_info(PcPciInfo *info)
>
> #define NameOp 0x08
> #define ScopeOp 0x10
> +#define BufferOp 0x11
> #define DeviceOp 0x82
>
> +#define EndTag 0x79
I would say we should use the values from
Table 6-162 Small Resource Items.
Wrap them in a function to get the full resource.
> +#define Decode16 0x1
> +#define Decode10 0x0
> +
This is the name from ASL, it's really _DEC value.
> static void
> build_header(GArray *linker, GArray *table_data,
> AcpiTableHeader *h, uint32_t sig, int len, uint8_t rev)
> @@ -406,6 +411,25 @@ static void build_append_int(GArray *table, uint32_t
> value)
> }
> }
>
> +static void build_prepend_int(GArray *array, uint32_t value)
> +{
> + GArray *data = build_alloc_array();
> +
> + build_append_int(data, value);
> + g_array_prepend_vals(array, data->data, data->len);
> + build_free_array(data);
> +}
> +
> +static void build_buffer(GArray *package, unsigned BufferSize)
> +{
> + uint32_t len = package->len > BufferSize ? package->len : BufferSize;
> +
> + /* TODO: buffer padding if BufferSize > actual buffer length */
Not sure what this means.
So assert here?
Or just make it work ...
> + build_prepend_int(package, len);
> + build_prepend_package_length(package, 0);
> + build_prepend_byte(package, BufferOp);
prepend is confusing.
Just do it like we do for methods:
build_append_and_cleanup_buffer(template, buffer);
> +}
> +
> static GArray *build_alloc_method(const char *name, uint8_t arg_count)
> {
> GArray *method = build_alloc_array();
> @@ -523,6 +547,14 @@ static uint32_t encodeEisaId(const char *str)
> build_free_array(name); \
> }
>
> +#define ACPI_BUFFER(ctx, name, min_size, ...) { \
Why pass in min_size?
the only reason we have it in existing code
was I wanted ACPI to be bit for bit compatible
with what seabios generated.
We can drop minsize everywhere ...
> + GArray *name = build_alloc_array(); \
> + __VA_ARGS__; \
> + build_buffer(name, min_size); \
> + build_append_array(ctx, name); \
> + build_free_array(name); \
> +}
> +
> #define ACPI_NAME(ctx, name) { \
> build_append_byte(ctx, NameOp); \
> build_append_nameseg(ctx, name); \
> @@ -541,6 +573,29 @@ static uint32_t encodeEisaId(const char *str)
> build_free_array(name); \
> }
>
> +#define ACPI_ENDTAG(ctx) { \
> + build_append_byte(ctx, EndTag); \
> + build_append_byte(ctx, 0); \
Confused.
what's going on with the checksum here?
What fills it in?
why don't we add in the correct byte straight away?
> +}
> +
> +#define ACPI_RESOURCE_TEMPLATE(ctx, name, ...) { \
> + ACPI_BUFFER(ctx, name, 0, \
> + __VA_ARGS__; \
> + ACPI_ENDTAG(name); \
Ugh.
Not worth the ugliness in my opinion.
Just add end tag explicitly.
> + ) \
> +}
> +
> +#define ACPI_IO(ctx, _DEC, _MIN_BASE, _MAX_BASE, _ALN, _LEN) { \
C spec says
— All identifiers that begin with an underscore and either an uppercase
letter or another
underscore are always reserved for any use.
— All identifiers that begin with an underscore are always reserved for
use as identifiers
so we try to avoid these.
> + build_append_byte(ctx, 0x47 /* IO port descriptor */); \
> + build_append_byte(ctx, _DEC); \
> + build_append_byte(ctx, _MIN_BASE & 0xff); \
> + build_append_byte(ctx, (_MIN_BASE >> 8) & 0xff); \
> + build_append_byte(ctx, _MAX_BASE & 0xff); \
> + build_append_byte(ctx, (_MAX_BASE >> 8) & 0xff); \
> + build_append_byte(ctx, _ALN); \
> + build_append_byte(ctx, _LEN); \
> +}
> +
> /* FACS */
> static void
> build_facs(GArray *table_data, GArray *linker, PcGuestInfo *guest_info)
> @@ -1084,6 +1139,13 @@ build_ssdt(GArray *table_data, GArray *linker,
> ACPI_SCOPE(sb_scope, PCI0,
> ACPI_DEVICE(PCI0, MRES,
> ACPI_NAME(MRES, "_HID"); ACPI_EISAID(MRES, "PNP0C02");
> + ACPI_NAME(MRES, "_CRS"); ACPI_RESOURCE_TEMPLATE(MRES, RESBUF,
> + ACPI_IO(RESBUF, Decode16,
> + pm->gpe0_blk, /* _MIN */
> + pm->gpe0_blk, /* _MAX */
> + 0x0, /* _ALN */
> + pm->gpe0_blk_len); /* _LEN */
> + );
> );
> );
Ugh, that's too tricky I'm afraid.
how about:
crs = build_alloc_array();
buf = build_alloc_buffer();
build_append_io(buf, .... );
build_append_and_cleanup_buffer(crs, buf);
make everything use static functions, not macros.
>
> --
> 1.7.1
