From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60936)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1Yieo2-00051Z-BW
	for qemu-devel@nongnu.org; Thu, 16 Apr 2015 04:05:51 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1Yienz-0006u7-3a
	for qemu-devel@nongnu.org; Thu, 16 Apr 2015 04:05:50 -0400
Received: from mx1.redhat.com ([209.132.183.28]:56058)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1Yieny-0006ti-SP
	for qemu-devel@nongnu.org; Thu, 16 Apr 2015 04:05:47 -0400
Received: from int-mx10.intmail.prod.int.phx2.redhat.com
	(int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3G85jI4015206
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256
	verify=FAIL)
	for <qemu-devel@nongnu.org>; Thu, 16 Apr 2015 04:05:45 -0400
Date: Thu, 16 Apr 2015 10:05:43 +0200
From: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <20150416100216-mutt-send-email-mst@redhat.com>
References: <1429017160-3583-1-git-send-email-kraxel@redhat.com>
	<20150414172749-mutt-send-email-mst@redhat.com>
	<1429107120.6219.15.camel@nilsson.home.kraxel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1429107120.6219.15.camel@nilsson.home.kraxel.org>
Subject: Re: [Qemu-devel] [PATCH 1/2] q35: implement SMRAM.D_LCK
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org

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
>