[coreboot-gerrit] Change in coreboot[master]: acpigen: Add ability to auto-generate _DSM Function 0

[Sugnan Prabhu S (Code Review)]

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, EricR Lai.
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56459 )

Change subject: acpigen: Add ability to auto-generate _DSM Function 0
......................................................................

Patch Set 5:

(1 comment)

File src/acpi/acpigen.c:

https://review.coreboot.org/c/coreboot/+/56459/comment/6b9f6da1_faf31f00 
PS5, Line 1657: 
              : 	for (size_t i = 1; i < id->count; i++) {
              : 		if (id->callbacks[i]) {
              : 			const size_t buffer_idx = i / (sizeof(uint8_t) * BITS_PER_BYTE);
              : 			const size_t bit_idx = i % (sizeof(uint8_t) * BITS_PER_BYTE);
              : 
              : 			set = true;
              : 			buffer[buffer_idx] |= BIT(bit_idx);
              : 		}
              : 	}
Hi Tim,

I tested this patch and found two problems with the current.

1. Following is the output generated with the current implementation and Return is getting called unconditionally without checking for function index to be 0.

If ((Local0 == ToUUID ("f21202bf-8f78-4dc6-a5b3-1f738e285ade")))
{
    ToInteger (Arg2, Local1)
    Return (Buffer (0x08)
    {
    	 0xFF, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00   // ........
    })
    If ((Local1 == One))
    {
            ...
    }

Following is expected
    If ((Local1 == Zero))
    {
    	Return (Buffer (0x08)
    	{
    		 0xFF, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00   // ........
    	})
    }

2. As per the ACPI spec, each bit in the 32bit(4bytes) buffer signifies the support for the function.

For example if we have define 9 callbacks with the 0th set to NULL, following is the expected output
    If ((Local1 == Zero))
    {
    	Return (Buffer (0x02)
    	{
    		 0xFF, 0x01
    	})
    }

I update the logic to the following and it is working as expected. Please let me know your thoughts.

static void acpigen_dsm_uuid_enum_functions(const struct dsm_uuid *id)
{
        const size_t bytes = (id->count / BITS_PER_BYTE) + ((id->count % BITS_PER_BYTE) > 0);
        uint8_t *buffer = alloca(bytes);
        bool set = false;
        int cb_index = 0;

for (size_t i = 0; i < bytes; i++) {
                for (size_t j = 0; j < BITS_PER_BYTE; j++) {
                        if (cb_index >= id->count)
                                break;

if (id->callbacks[cb_index++]) {
                                set = true;
                                buffer[i] |= BIT(j);
                        }
                }
        }

if (set)
                buffer[0] |= BIT(0);

acpigen_write_if_lequal_op_int(LOCAL1_OP, 0);
        acpigen_write_return_byte_buffer(buffer, bytes);
        acpigen_pop_len();      /* If */
}