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

[Michael S. Tsirkin]

On Wed, Apr 15, 2015 at 04:12:00PM +0200, Gerd Hoffmann wrote:
>   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.

I'd prefer a complete patch to review.
Let's just set the correct wmask values directly.

> 
> > >  
> > >      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
>