Re: [Qemu-devel] [PATCH 1/2] q35: implement SMRAM.D_LCK

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> >      d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT;
> > +    d->wmask[MCH_HOST_BRIDGE_SMRAM] = 0xff;
> 
> Is this right? I see a bunch of reserved bits etc there.

Restores the state we had before the guest flipped the lock bit.

Entriely possible that we should have a non-0xff wmask in the first
place, I'll look into that, but it's unrelated to lock bit handling and
thus something for another patch.

> > +    d->wmask[MCH_HOST_BRIDGE_ESMRAMC] = 0xff;
> 
> Doesn't this mean we need to reset this register now?

Again, this is something not related to the lock bit implementation,
probably the patch adding esramc support should have added this too.

I'll have a look, probably will cook up a incremental fix paolo can
squash in then.

> >  
> >      mch_update(mch);
> >  }
> 
> Don't you also need to clear D_LCK?

Setting MCH_HOST_BRIDGE_SMRAM to MCH_HOST_BRIDGE_SMRAM_DEFAULT does
that.

Also see 2/2 with the test case which shows lock+unlock works correctly.

cheers,
  Gerd