From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40409) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnSB1-0001Bh-6u for qemu-devel@nongnu.org; Wed, 29 Apr 2015 09:37:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YnSAw-00007j-3Z for qemu-devel@nongnu.org; Wed, 29 Apr 2015 09:37:23 -0400 Received: from mail-pd0-f172.google.com ([209.85.192.172]:33233) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YnSAv-00007P-Sf for qemu-devel@nongnu.org; Wed, 29 Apr 2015 09:37:18 -0400 Received: by pdbnk13 with SMTP id nk13so28625855pdb.0 for ; Wed, 29 Apr 2015 06:37:17 -0700 (PDT) Message-ID: <5540DE87.1020204@linaro.org> Date: Wed, 29 Apr 2015 21:37:11 +0800 From: Shannon Zhao MIME-Version: 1.0 References: <1429104309-3844-1-git-send-email-zhaoshenglong@huawei.com> <1429104309-3844-20-git-send-email-zhaoshenglong@huawei.com> <20150428104225.4d5ebab0@nial.brq.redhat.com> <20150428104449-mutt-send-email-mst@redhat.com> <553F4D78.5070507@huawei.com> <20150428115407.66991856@nial.brq.redhat.com> <553F83B5.5020002@huawei.com> <20150428171310.76d6cd77@nial.brq.redhat.com> <20150428174836-mutt-send-email-mst@redhat.com> <55404C04.40204@huawei.com> <20150429104756.569f728d@nial.brq.redhat.com> In-Reply-To: <20150429104756.569f728d@nial.brq.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 19/20] hw/arm/virt-acpi-build: Add PCIe controller in ACPI DSDT table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , Shannon Zhao Cc: peter.maydell@linaro.org, hangaohuai@huawei.com, "Michael S. Tsirkin" , a.spyridakis@virtualopensystems.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, alex.bennee@linaro.org, hanjun.guo@linaro.org, pbonzini@redhat.com, lersek@redhat.com, christoffer.dall@linaro.org On 2015/4/29 16:47, Igor Mammedov wrote: > On Wed, 29 Apr 2015 11:12:04 +0800 > Shannon Zhao 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. /* 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 */ >>>>>>>> >>>>>>>> . >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> . >>>>>> >>>>> >>>>> >>> >>> . >>> >> >> >