From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Hongbo Zhang <hongbo.zhang@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>,
Radoslaw Biernacki <radoslaw.biernacki@linaro.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
QEMU Developers <qemu-devel@nongnu.org>,
Leif Lindholm <leif.lindholm@linaro.org>,
qemu-arm <qemu-arm@nongnu.org>, Laszlo Ersek <lersek@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
Date: Mon, 17 Jun 2019 13:08:31 +0200 [thread overview]
Message-ID: <cb17ecf4-d321-2f71-7528-2fd5aec2cccd@redhat.com> (raw)
In-Reply-To: <CAHmQWvAuQ9mwXA5uO3ShYyvP5ywOrck5kU4gPD_kGh5S39Va2w@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 7355 bytes --]
On 6/16/19 1:41 PM, Hongbo Zhang wrote:
> On Mon, 3 Jun 2019 at 18:54, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Hi Hongbo, Ard.
>>
>> On 4/18/19 6:04 AM, Hongbo Zhang wrote:
>>> Following the previous patch, this patch adds peripheral devices to the
>>> newly introduced SBSA-ref machine.
>>>
>>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>>> ---
>>> hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 451 insertions(+)
>>>
>>> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
[...]
>>> +static void create_one_flash(const char *name, hwaddr flashbase,
>>> + hwaddr flashsize, const char *file,
>>> + MemoryRegion *sysmem)
>>> +{
>>> + /*
>>> + * Create and map a single flash device. We use the same
>>> + * parameters as the flash devices on the Versatile Express board.
>>> + */
>>> + DriveInfo *dinfo = drive_get_next(IF_PFLASH);
>>> + DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
>>
>> Please use TYPE_PFLASH_CFI01 instead of "cfi.pflash01".
>>
>> I wanted to ask "does it has to be CFI01?" because this device model is
>> in bad shape, but I guess I answered myself looking at the EDK2 platform
>> code:
>>
>> - P30_CFI_ADDR_VENDOR_ID is not used
>> - NorFlashDxe::NorFlashReadCfiData() is not implemented
>> - All commands in NorFlashDxe uses:
>> SEND_NOR_COMMAND(..., P30_CMD_...)
>> which are specific to the Intel P30 Nor flash family (CFI01).
>>
>>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> + const uint64_t sectorlength = 256 * 1024;
>>> +
>>> + if (dinfo) {
>>> + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
>>> + &error_abort);
>>> + }
>>> +
>>> + qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
>>> + qdev_prop_set_uint64(dev, "sector-length", sectorlength);
>>> + qdev_prop_set_uint8(dev, "width", 4);
>>> + qdev_prop_set_uint8(dev, "device-width", 2);
>>> + qdev_prop_set_bit(dev, "big-endian", false);
>>> + qdev_prop_set_uint16(dev, "id0", 0x89);
>>> + qdev_prop_set_uint16(dev, "id1", 0x18);
>>> + qdev_prop_set_uint16(dev, "id2", 0x00);
>>> + qdev_prop_set_uint16(dev, "id3", 0x00);
>>> + qdev_prop_set_string(dev, "name", name);
>>> + qdev_init_nofail(dev);
>>> +
>>> + memory_region_add_subregion(sysmem, flashbase,
>>> + sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
>>> +
>>> + if (file) {
>>> + char *fn;
>>> + int image_size;
>>> +
>>> + if (drive_get(IF_PFLASH, 0, 0)) {
>>> + error_report("The contents of the first flash device may be "
>>> + "specified with -bios or with -drive if=pflash... "
>>> + "but you cannot use both options at once");
>>> + exit(1);
>>> + }
>>> + fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
>>> + if (!fn) {
>>> + error_report("Could not find ROM image '%s'", file);
>>> + exit(1);
>>> + }
>>> + image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
>>> + g_free(fn);
>>> + if (image_size < 0) {
>>> + error_report("Could not load ROM image '%s'", file);
>>> + exit(1);
>>> + }
>>> + }
>>> +}
>>> +
>>> +static void create_flash(const SBSAMachineState *vms,
>>> + MemoryRegion *sysmem,
>>> + MemoryRegion *secure_sysmem)
>>> +{
>>> + /*
>>> + * Create one secure and nonsecure flash devices to fill SBSA_FLASH
>>> + * space in the memmap, file passed via -bios goes in the first one.
>>> + */
>>> + hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
>>> + hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
>>> +
>>> + create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
>>> + bios_name, secure_sysmem);
>>> + create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
>>> + NULL, sysmem);
>>
>> static const MemMapEntry base_memmap[] = {
>> /* Space up to 0x8000000 is reserved for a boot ROM */
>> [VIRT_FLASH] = { 0, 0x08000000 },
>>
> Hi Philippe,
> Thank you for the long comments.
> Some parts of this machine are based on the 'virt' machine, but I use
> this flash memory map:
> [SBSA_FLASH] = { 0, 0x20000000 },
> that are 256M *2 flashes.
> Franky I didn't consider the product part number etc, just use the
> original design in 'virt' and change the size large enough as I think.
I guess we are very lucky... The Micron PC28F00BP33EF is a 2Gib
symmetrical blocks NOR flash, and the only CFI01 one I could find a
datasheet :)
"The 2Gb device employs a virtual chip enable feature, which combines
two 1Gb die with a common chip enable".
> Peter, Ard, do we need more considerations here?
>
>> So you are creating 2 identical flashes of 128MiB/2 = 64 MiB each which
>> are the biggest flash you can have:
>>
>> "The P30 family provides density upgrades from 64-Mbit through
>> 512-Mbit." On Intel, the 512-Mib case is particular in that it is built
>> of 2x 256-Mib on the same die, with a virtual chip enable. It is simpler
>> to use a Micron or Numonyx model.
>>
>> I plan to use a whitelist of supported (and tested...) models, the one
>> you use seems the Micron JS28F512P30EF ('E' for 'Symetrically Blocked',
>> since the current model doesn't support bottom/top blocks layout), or in
>> short: 28F512P30E.
>> Ard, is that OK?
>>
>> Checking EDK2 git history, the driver is part of ArmPlatformPkg,
>> imported in commit 1d5d0ae92d9541, based on 'Versatile Express'.
>>
>> On the Versatile Express and the RealView Emulation Baseboard user
>> guides, I only find reference of "64MB of NOR flash" with no specific model.
>>
>> Peter, do you have physical access to tell me what flashes are used on
>> real hardware? I googled for Linux console boot log but the kernel
>> doesn't seem to care about detecting/mapping the flash.
>>
>> QEMU added the flash to the Versatile board in commit 964c695a54ceda3a,
>> with the following description:
>>
>> - add support for the 64MB NOR CFI01 flash available at
>> 0x34000000 on the versatilepb board
>>
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0225d/BBAJIHEC.html
>>
>> However on this link I only see "SSMC Chip Select 1, normally NOR flash
>> (During boot remapping, this can be NOR flash, Disk-on-Chip, or static
>> expansion memory)". Again, nothing specific (which makes sense, why
>> restrict the users to a particuliar family, as long as the pinout matches).
>>
>> The Manufacturer/Device ID used in QEMU (0x0089, 0x0018) correspond to
>> the Micron 28F128J3D (128-Mbit, 128 symmetrical blocks of 128-KiB).
>> Neither the flash size (64 vs 16) nor the block size (256 vs 128) match.
>>
>> The safer fix here is to find a CFI01 flash of 256 sectors of 256-KiB
>> and update the Manufacturer/Device IDs in QEMU. Luckily this matches the
>> 28F512P30E cited previously :)
>>
>> Regards,
>>
>> Phil.
[...]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2019-06-17 11:10 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-18 4:04 [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine Hongbo Zhang
2019-04-18 4:04 ` Hongbo Zhang
2019-04-18 4:04 ` [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part Hongbo Zhang
2019-04-18 4:04 ` Hongbo Zhang
2019-04-30 14:03 ` Peter Maydell
2019-04-30 14:03 ` Peter Maydell
2019-05-08 11:22 ` Hongbo Zhang
2019-05-09 14:27 ` Igor Mammedov
2019-05-09 14:40 ` Peter Maydell
2019-04-18 4:04 ` [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part Hongbo Zhang
2019-04-18 4:04 ` Hongbo Zhang
2019-04-30 14:16 ` Peter Maydell
2019-04-30 14:16 ` Peter Maydell
2019-05-08 11:30 ` Hongbo Zhang
2019-05-08 17:48 ` Radoslaw Biernacki
2019-05-09 8:46 ` Peter Maydell
2019-05-08 13:59 ` Markus Armbruster
2019-06-02 3:16 ` Hongbo Zhang
2019-06-03 10:54 ` Philippe Mathieu-Daudé
2019-06-16 11:41 ` Hongbo Zhang
2019-06-17 11:08 ` Philippe Mathieu-Daudé [this message]
2019-06-17 2:44 ` Hongbo Zhang
2019-04-30 14:18 ` [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine Peter Maydell
2019-04-30 14:18 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cb17ecf4-d321-2f71-7528-2fd5aec2cccd@redhat.com \
--to=philmd@redhat.com \
--cc=ard.biesheuvel@linaro.org \
--cc=hongbo.zhang@linaro.org \
--cc=leif.lindholm@linaro.org \
--cc=lersek@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=radoslaw.biernacki@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).