Re: [Qemu-devel] [PATCH v2 3/4] nvdimm: add a boolean option "restrict"

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1479 bytes --]

On Thu, Jun 08, 2017 at 02:39:48PM +0800, Haozhong Zhang wrote:
> On 06/07/17 16:27 +0100, Stefan Hajnoczi wrote:
> > On Tue, Jun 06, 2017 at 03:22:28PM +0800, Haozhong Zhang wrote:
> > > If a vNVDIMM device is not backed by a DAX device and its "restrict"
> > > option is enabled, bit 3 of state flags in its region mapping
> > > structure will be set, in order to notify the guest of the lack of
> > > write persistence guarantee. Once this bit is set, the guest OS may
> > > mark the vNVDIMM device as read-only.
> > > 
> > > This option is disabled by default for backwards compatibility. It's
> > > recommended to enable for the formal usage.
> > 
> > Good idea.  I think the following is cleaner:
> > 
> > DEFINE_PROP_ON_OFF_AUTO("readonly") on the 'nvdimm' device.  The
> > following states are available:
> > 
> >  * 'on' - ACPI_NFIT_MEM_NOT_ARMED is set
> >  * 'off' - ACPI_NFIT_MEM_NOT_ARMED is clear
> >  * 'auto' - ACPI_NFIT_MEM_NOT_ARMED set if backend is not persistent
> > 
> > This new property defaults to 'auto'.  Machine types older than
> > pc-i440fx-2.10 and pc-q35-2.10 default to 'on'.
> 
> I think the the name "readonly" is not precise, because QEMU only sets
> one bit and does not prevent guest writes. It's guest decision to
> treat the vNVDIMM devices as read-only (e.g. Linux kernel).
> 
> We may use "unsafe-write" instead.

I agree, "readonly" isn't accurate.  I would use the NFIT terminology
and call it "armed".

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]