Re: [PATCH RESEND v2] block/nvme: introduce PMR support from NVMe 1.4 spec

[Andrzej Jakowski]

On 3/17/20 4:23 AM, Stefan Hajnoczi wrote:
>> Code is posted here
>> https://github.com/AndrzejJakowski/qemu/commit/3a7762a1d13ff1543d1da430748eb24e38faab6f
>>
>> QEMU command line:
>>
>> # below are just relevant pieces of configuration, other stuff omitted
>> # tried different setting (e.g. pmem=on and pmem=off)
>>
>> ./x86_64-softmmu/qemu-system-x86_64 ... \
>> -object 
>> memory-backend-file,id=mem1,share=off,pmem=on,mem-path=../nvme_pmr.bin,size=$((1*1024*1024))
>>  \
> share=off is MAP_PRIVATE.  If persistence is desired then share=on
> should be used.
> 
> However, this shouldn't affect "system_reset" behavior since the QEMU
> process still has the same mapped file open.
> 

Hi Stefan,

Thx!! share=off setting was the problem. I confirmed with my simple test
that persistence is achieved.
I didn't find API to perform flush (msync). Any suggestion what function to use?

Given that host memory backend is working I think my patch is almost ready for 
resubmission -- let me know if there are any other comments.

Andrzej

>> -drive file=../nvme.bin,format=raw,if=none,id=nvme_emulated \
>> -device nvme,drive=nvme_emulated,serial="test serial",pmrdev=mem1
>>
>> In VM:
>> My persisent memory region is exposed PCI BAR
>> Region 2: Memory at fe000000 (64-bit, prefetchable) [size=1M]
>>
>> So I perform reads/writes from/to following adress 0xfe000000 (decimal 
>> 4261412864)
>>
>> dd if=test.bin of=/dev/mem bs=1 count=30 seek=4261412864
>> dd if=/dev/mem of=test1.bin bs=1 count=30 skip=4261412864
> Did you verify that the guest kernel is really accessing the BAR?  I
> remember that distro kernels often ship with options that make
> /dev/mem of limited use because it's considered insecure.
> 
>> On VMM I didn't observe that backing file has been updated and after power 
>> cycling VM
>> I see old junk when reading PMR region.
> Did you check that the pmrdev mmap region contains the data the guest
> wrote before power cycling?
> 
>> Also from include/qemu/pmem.h it looks like pmem_persist() will cause qemu 
>> to exit
>> if libpmem is not installed:
> The libpmem support only needs to be used when the pmem=on option was
> given.  If there isn't a physical pmem device then it doesn't need to
> be used.
> 
> Stefan