Re: [PATCH kvmtool v3] Add emulation for CFI compatible flash memory

["André Przywara" <andre.przywara@arm.com>]

On 15/04/2020 17:20, Ard Biesheuvel wrote:
> On Wed, 15 Apr 2020 at 18:11, André Przywara <andre.przywara@arm.com> wrote:
>>
>> On 15/04/2020 16:55, Ard Biesheuvel wrote:
>>> On Wed, 15 Apr 2020 at 17:43, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Tue, 7 Apr 2020 at 17:15, Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I've tested this patch by running badblocks and fio on a flash device inside a
>>>>> guest, everything worked as expected.
>>>>>
>>>>> I've also looked at the flowcharts for device operation from Intel Application
>>>>> Note 646, pages 12-21, and they seem implemented correctly.
>>>>>
>>>>> A few minor issues below.
>>>>>
>>>>> On 2/21/20 4:55 PM, Andre Przywara wrote:
>>>>>> From: Raphael Gault <raphael.gault@arm.com>
>>>>>>
>>>>>> The EDK II UEFI firmware implementation requires some storage for the EFI
>>>>>> variables, which is typically some flash storage.
>>>>>> Since this is already supported on the EDK II side, we add a CFI flash
>>>>>> emulation to kvmtool.
>>>>>> This is backed by a file, specified via the --flash or -F command line
>>>>>> option. Any flash writes done by the guest will immediately be reflected
>>>>>> into this file (kvmtool mmap's the file).
>>>>>> The flash will be limited to the nearest power-of-2 size, so only the
>>>>>> first 2 MB of a 3 MB file will be used.
>>>>>>
>>>>>> This implements a CFI flash using the "Intel/Sharp extended command
>>>>>> set", as specified in:
>>>>>> - JEDEC JESD68.01
>>>>>> - JEDEC JEP137B
>>>>>> - Intel Application Note 646
>>>>>> Some gaps in those specs have been filled by looking at real devices and
>>>>>> other implementations (QEMU, Linux kernel driver).
>>>>>>
>>>>>> At the moment this relies on DT to advertise the base address of the
>>>>>> flash memory (mapped into the MMIO address space) and is only enabled
>>>>>> for ARM/ARM64. The emulation itself is architecture agnostic, though.
>>>>>>
>>>>>> This is one missing piece toward a working UEFI boot with kvmtool on
>>>>>> ARM guests, the other is to provide writable PCI BARs, which is WIP.
>>>>>>
>>>>
>>>> I have given this a spin with UEFI built for kvmtool, and it appears
>>>> to be working correctly. However, I noticed that it is intolerably
>>>> slow, which seems to be caused by the fact that both array mode and
>>>> command mode (or whatever it is called in the CFI spec) are fully
>>>> emulated, whereas in the QEMU implementation (for instance), the
>>>> region is actually exposed to the guest using a read-only KVM memslot
>>>> in array mode, and so the read accesses are made natively.
>>>>
>>>> It is also causing problems in the UEFI implementation, as we can no
>>>> longer use unaligned accesses to read from the region, which is
>>>> something the code currently relies on (and which works fine on actual
>>>> hardware as long as you use normal non-cacheable mappings)
>>>>
>>>
>>> Actually, the issue is not alignment. The issue is with instructions
>>> with multiple outputs, which means you cannot do an ordinary memcpy()
>>> from the NOR region using ldp instructions, aligned or not.
>>
>> Yes, we traced that down to an "ldrb with post-inc", in the memcpy code.
>> My suggestion was to provide a version of memcpy_{from,to}_io(), as
>> Linux does, which only uses MMIO accessors to avoid "fancy" instructions.
>>
> 
> That is possible, and the impact on the code is manageable, given the
> modular nature of EDK2.
> 
>> Back at this point I was challenging the idea of accessing a flash
>> device with a normal memory mapping, because of it failing when being in
>> some query mode. Do you know of any best practices for flash mappings?
>> Are two mappings common?
>>
> 
> In the QEMU port of EDK2, we use normal non-cacheable for the first
> flash device, which contains the executable image, and is not
> updatable by the guest. The second flash bank is used for the variable
> store, and is actually mapped as a device all the time.
> 
> Another thing I just realized is that you cannot fetch instructions
> from an emulated flash device either, so to execute from NOR flash,
> you will need a true memory mapping as well.

Wait, did you put the whole of EDK-2 image in the flash? My assumption
(and testing) was to use

$ lkvm run -f KVMTOOL_EFI.fd --flash just_the_variables.img

Hence my ignorance about performance, because it would just be a few
bytes written/read. -f loads the firmware image into guest RAM.

> So in summary, I think the mode switch is needed to be generally
> useful, even if the current approach is sufficient for (slow)
> read/write using special memory accessors.

Well,in hindsight I regret pursuing this whole flash emulation approach
in kvmtool in the first place. Just some magic "this memory region is
persistent" (mmapping a file and presenting as a memslot) would be
*much* easier on the kvmtool side. It just seems that there wasn't any
good DT binding or existing device class for this (to my surprise), or
at least not one without issues. And then EDK-2 had this CFI flash
support already, so we figured this should be the way to go. We just
need some emulation code ... months later ...

So do you know of some persistent storage device we could use? This
would come at the cost of adding support to EDK-2, but I guess it should
be straight-forward given the simple semantic?

Cheers,
Andre

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel