On Fri, Feb 09, 2018 at 10:57:26PM +0800, Haozhong Zhang wrote:
> On 02/09/18 14:27 +0000, Stefan Hajnoczi wrote:
> > On Wed, Feb 07, 2018 at 03:33:27PM +0800, Haozhong Zhang wrote:
> > > @@ -156,11 +157,17 @@ static void nvdimm_write_label_data(NVDIMMDevice
> > > *nvdimm, const void *buf,
> > > {
> > > MemoryRegion *mr;
> > > PCDIMMDevice *dimm = PC_DIMM(nvdimm);
> > > + bool is_pmem = object_property_get_bool(OBJECT(dimm->hostmem),
> > > + "pmem", NULL);
> > > uint64_t backend_offset;
> > >
> > > nvdimm_validate_rw_label_data(nvdimm, size, offset);
> > >
> > > - memcpy(nvdimm->label_data + offset, buf, size);
> > > + if (!is_pmem) {
> > > + memcpy(nvdimm->label_data + offset, buf, size);
> > > + } else {
> > > + pmem_memcpy_persist(nvdimm->label_data + offset, buf, size);
> > > + }
> >
> > Is this enough to prevent label corruption in case of power failure?
> >
> > pmem_memcpy_persist() is not atomic. Power failure can result in a mix
> > of the old and new label data.
> >
> > If we want this operation to be 100% safe there needs to be some kind of
> > update protocol that makes the change atomic, like a Label A and Label B
> > area with a single Label Index field that can be updated atomically to
> > point to the active Label A/B area.
>
> All this patch series is to guarantee: if the guest is still alive and
> running, all its previous writes to pmem, which were performed by
> QEMU, will be still persistent on pmem.
>
> If a power failure happens before QEMU returns to the guest, e.g., in
> the middle of above pmem_memcpy_persist(), yes, the guest label data
> may be in an inconsistent state, but the guest also has no chance to
> progress. And, that is what could happen in the non-virtualization
> environment as well, and it's the responsibility of the (guest) SW to
> defend such failures, e.g., by the protocol you mentioned.Thanks for explaining! I thought the atomic update is done by the ACPI _LSW DSM method but it turns out the driver must do it. For anyone else who is interested, a detailed description of the Label Storage Area is on page 652 of the UEFI Specification version 2.7 and the Linux driver implements the atomic update algorithm in drivers/nvdimm/label.c:__pmem_label_update(). Stefan
signature.asc
Description: PGP signature
