[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 22/36] hw/arm/allwinner-h3: add SDRAM controller device
From: |
Peter Maydell |
Subject: |
Re: [PULL 22/36] hw/arm/allwinner-h3: add SDRAM controller device |
Date: |
Fri, 20 Mar 2020 15:46:08 +0000 |
On Thu, 12 Mar 2020 at 16:45, Peter Maydell <address@hidden> wrote:
>
> From: Niek Linnenbank <address@hidden>
>
> In the Allwinner H3 SoC the SDRAM controller is responsible
> for interfacing with the external Synchronous Dynamic Random
> Access Memory (SDRAM). Types of memory that the SDRAM controller
> supports are DDR2/DDR3 and capacities of up to 2GiB. This commit
> adds emulation support of the Allwinner H3 SDRAM controller.
Hi; Coverity has flagged a possible issue with this patch
(CID 1421912):
> +static void allwinner_h3_dramc_map_rows(AwH3DramCtlState *s, uint8_t
> row_bits,
> + uint8_t bank_bits, uint16_t
> page_size)
> +{
> + /*
> + * This function simulates row addressing behavior when bootloader
> + * software attempts to detect the amount of available SDRAM. In U-Boot
> + * the controller is configured with the widest row addressing available.
> + * Then a pattern is written to RAM at an offset on the row boundary
> size.
> + * If the value read back equals the value read back from the
> + * start of RAM, the bootloader knows the amount of row bits.
> + *
> + * This function inserts a mirrored memory region when the configured row
> + * bits are not matching the actual emulated memory, to simulate the
> + * same behavior on hardware as expected by the bootloader.
> + */
> + uint8_t row_bits_actual = 0;
> +
> + /* Calculate the actual row bits using the ram_size property */
> + for (uint8_t i = 8; i < 12; i++) {
> + if (1 << i == s->ram_size) {
> + row_bits_actual = i + 3;
> + break;
> + }
> + }
> +
> + if (s->ram_size == (1 << (row_bits - 3))) {
> + /* When row bits is the expected value, remove the mirror */
> + memory_region_set_enabled(&s->row_mirror_alias, false);
> + trace_allwinner_h3_dramc_rowmirror_disable();
> +
> + } else if (row_bits_actual) {
> + /* Row bits not matching ram_size, install the rows mirror */
> + hwaddr row_mirror = s->ram_addr + ((1 << (row_bits_actual +
> + bank_bits)) * page_size);
In this calculation we do the multiply as a signed 32-bit operation,
which then gets sign-extended to 64 bits for the addition; that
means that if the multiply result is greater than 0x7fffffff then
the upper bits of the result will all be 1s. Is this a "can't happen"
situation, or should we be using "1ULL" to force a 64-bit multiply?
thanks
-- PMM
- Re: [PULL 21/36] hw/arm/allwinner-h3: add Boot ROM support, (continued)
[PULL 24/36] tests/boot_linux_console: Add a quick test for the OrangePi PC board, Peter Maydell, 2020/03/12
[PULL 25/36] tests/boot_linux_console: Add initrd test for the Orange Pi PC board, Peter Maydell, 2020/03/12
[PULL 26/36] tests/boot_linux_console: Add a SD card test for the OrangePi PC board, Peter Maydell, 2020/03/12
[PULL 20/36] hw/arm/allwinner-h3: add EMAC ethernet device, Peter Maydell, 2020/03/12
[PULL 23/36] hw/arm/allwinner: add RTC device support, Peter Maydell, 2020/03/12
[PULL 28/36] tests/boot_linux_console: Test booting NetBSD via U-Boot on OrangePi PC, Peter Maydell, 2020/03/12
[PULL 31/36] hw/arm/virt: Introduce VirtGICType enum type, Peter Maydell, 2020/03/12
[PULL 22/36] hw/arm/allwinner-h3: add SDRAM controller device, Peter Maydell, 2020/03/12
- Re: [PULL 22/36] hw/arm/allwinner-h3: add SDRAM controller device,
Peter Maydell <=
[PULL 19/36] hw/arm/allwinner: add SD/MMC host controller, Peter Maydell, 2020/03/12
[PULL 27/36] tests/boot_linux_console: Add a SLOW test booting Ubuntu on OrangePi PC, Peter Maydell, 2020/03/12
[PULL 33/36] target/arm/kvm: Let kvm_arm_vgic_probe() return a bitmap, Peter Maydell, 2020/03/12
[PULL 30/36] hw/arm/virt: Document 'max' value in gic-version property description, Peter Maydell, 2020/03/12
[PULL 34/36] hw/arm/virt: kvm: Restructure finalize_gic_version(), Peter Maydell, 2020/03/12
[PULL 35/36] hw/arm/virt: kvm: allow gicv3 by default if v2 cannot work, Peter Maydell, 2020/03/12
[PULL 29/36] docs: add Orange Pi PC document, Peter Maydell, 2020/03/12
[PULL 32/36] hw/arm/virt: Introduce finalize_gic_version(), Peter Maydell, 2020/03/12