On Wed, 29 Apr 2015 21:37:11 +0800 Shannon Zhao <shannon.z...@linaro.org> wrote:
> > > On 2015/4/29 16:47, Igor Mammedov wrote: > > On Wed, 29 Apr 2015 11:12:04 +0800 > > Shannon Zhao <zhaoshengl...@huawei.com> wrote: > > > >> On 2015/4/28 23:54, Michael S. Tsirkin wrote: > >>> On Tue, Apr 28, 2015 at 05:13:10PM +0200, Igor Mammedov wrote: > >>>>> Here I need to set the value of buffer to 1 or 0, we could > >>>>> createbitfield, but if we want to set the value to non one or zero and > >>>>> the BufferSize is large, how could we do? CreateByteField? It's a little > >>>>> complex for user. > >>>> that's what one would have to do writing it in ASL if bits > >>>> are flipped on/off dynamically. > >>>> > >>>> In ASL you also can declare buffer with static initializer > >>>> > >>>> Buffer (0x01) { 0x03 } > >>>> > >>>> and compiler is smart enough to set appropriate bits but it doesn't > >>>> allow you to do so with large values. For example: > >>>> > >>>> Buffer (0x01) { 0xAABBCCDD } > >>>> > >>>> gives error: > >>>> Error 6139 - Constant out of range ^ (0xAABBCCDD, allowable: 0x0-0xFF) > >>>> > >>>> If one wants to manipulate specific fields in Buffer, ASL has > >>>> a bunch of CreateFOOField operators, so lets follow spec and use > >>>> API consistently to avoid confusion. > >>>> > >>>> BTW: > >>>> packaging value as int (even without prefix) is wrong since > >>>> its LE encoding will shuffle bytes and you won't get bits in > >>>> positions that you expect if value is more than 1 byte. > >>> > >>> I don't care about ASL, we are writing in C > >>> But AML is same: > >>> DefBuffer := BufferOp PkgLength BufferSize ByteList > >>> BufferOp := 0x11 > >>> BufferSize := TermArg => Integer > >>> > >>> So really just a bytelist. > >>> We don't have any users for aml_buffer, maybe just add > >>> const uint8_t *bytes, unsigned len as parameters. > >>> > >> > >> Agree. It's consistent with the spec. If want to modify the value, could > >> use CreateFOOField. > >> > >> So use following fuction to initialize Buffer? > >> > >> /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */ > >> Aml *aml_buffer(int buffer_size, uint8_t *byte_list) > >> { > >> int i; > >> Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER); > >> for (i = 0; i < buffer_size; i++) { > >> build_append_byte(var->buf, *(byte_list + i)); > >> } > >> return var; > >> } > > maybe > > > > Aml *aml_buffer_initialized(int buffer_size, uint8_t *byte_list); > > Aml *aml_buffer(int buffer_size); > > > > the second one is needed for implementing code like: > > Name(BUFF, Buffer(4){}) // Create SerialBus data buffer as BUFF > > CreateByteField(BUFF, 0x00, STAT) // STAT = Status (Byte) > > CreateWordField(BUFF, 0x02, DATA) // DATA = Data (Byte) > > > > and could reuse aml_buffer_initialized() to reserve space. > > > > maybe we could use only one. For the uninitialized buffer we can pass > byte_list as NULL and within aml_buffer check byte_list, if byte_list is > NULL, just reserve space. works for me with doc comment > > /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */ > Aml *aml_buffer(int buffer_size, uint8_t *byte_list) > { > int i; > Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER); > > for (i = 0; i < buffer_size; i++) { > if (byte_list == NULL) > build_append_byte(var->buf, 0); > else > build_append_byte(var->buf, *(byte_list + i)); > } > > return var; > } > > >> > >>> Would that be enough? > >>> > >>> > >>>>> > >>>>>> ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0))); > >>>>>> aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1))); > >>>>>> ... > >>>>>> /* create bit field for every supported function if supported */ > >>>>>> ... > >>>>>> aml_append(method, aml_return(aml_name("RET"))); > >>>>>> > >>>>>> > >>>>>>> > >>>>>>>>> > >>>>>>>>>> + aml_append(ifctx1, aml_return(buf)); > >>>>>>>>>> + aml_append(ifctx, ifctx1); > >>>>>>>>>> + aml_append(method, ifctx); > >>>>>>>>>> + > >>>>>>>>>> + buf = aml_buffer(); > >>>>>>>>>> + build_append_int_noprefix(buf->buf, 0x00, 1); > >>>>>>>>>> + aml_append(method, aml_return(buf)); > >>>>>>>>>> + aml_append(dev, method); > >>>>>>>>>> + > >>>>>>>>>> + Aml *dev_rp0 = aml_device("%s", "RP0"); > >>>>>>>>>> + aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0))); > >>>>>>>>>> + aml_append(dev, dev_rp0); > >>>>>>>>>> + aml_append(scope, dev); > >>>>>>>>>> +} > >>>>>>>>>> + > >>>>>>>>>> /* RSDP */ > >>>>>>>>>> static GArray * > >>>>>>>>>> build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt) > >>>>>>>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, > >>>>>>>>>> VirtGuestInfo *guest_info) > >>>>>>>>>> acpi_dsdt_add_flash(scope, info->flash_memmap); > >>>>>>>>>> acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap, > >>>>>>>>>> info->virtio_mmio_irq, info->virtio_mmio_num); > >>>>>>>>>> + acpi_dsdt_add_pci(scope, guest_info->pcie_info); > >>>>>>>>>> + > >>>>>>>>>> aml_append(dsdt, scope); > >>>>>>>>>> > >>>>>>>>>> /* copy AML table into ACPI tables blob and patch header > >>>>>>>>>> there */ > >>>>>>>> > >>>>>>>> . > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>>>> . > >>>>>> > >>>>> > >>>>> > >>> > >>> . > >>> > >> > >> > >