From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paul Durrant <Paul.Durrant@citrix.com>
Subject: Re: [PATCH v7 07/15] x86/hvm: unify stdvga mmio
 intercept with standard mmio intercept
Date: Thu, 9 Jul 2015 16:12:02 +0000
Message-ID: <9AAE0902D5BC7E449B7C8E4E778ABCD02F4B6F81@AMSPEX01CL02.citrite.net>
References: <1436447455-11524-1-git-send-email-paul.durrant@citrix.com>
	<1436447455-11524-8-git-send-email-paul.durrant@citrix.com>
	<559EB057020000780008EEE8@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <559EB057020000780008EEE8@mail.emea.novell.com>
Content-Language: en-US
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: "Keir (Xen.org)" <keir@xen.org>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 09 July 2015 16:33
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org; Keir (Xen.org)
> Subject: Re: [PATCH v7 07/15] x86/hvm: unify stdvga mmio intercept with
> standard mmio intercept
> 
> >>> On 09.07.15 at 15:10, <paul.durrant@citrix.com> wrote:
> > @@ -424,8 +426,22 @@ static void stdvga_mem_writeb(uint64_t addr,
> uint32_t val)
> >      }
> >  }
> >
> > -static void stdvga_mem_write(uint64_t addr, uint64_t data, uint64_t size)
> > +static int stdvga_mem_write(const struct hvm_io_handler *handler,
> > +                            uint64_t addr, uint32_t size,
> > +                            uint64_t data)
> >  {
> > +    struct hvm_hw_stdvga *s = &current->domain-
> >arch.hvm_domain.stdvga;
> > +    ioreq_t p = { .type = IOREQ_TYPE_COPY,
> > +                  .addr = addr,
> > +                  .size = size,
> > +                  .count = 1,
> > +                  .dir = IOREQ_WRITE,
> > +                  .data = data,
> > +                };
> 
> Indentation (still - I know I pointed this out on v6, just perhaps at
> another example). See e.g. the context of the earlier change to the
> beginning of hvm_mmio_internal() in this patch for how this should
> look like.

It has to be something my emacs is doing then; I can't see any brokenness.

> 
> > -int stdvga_intercept_mmio(ioreq_t *p)
> > +static bool_t stdvga_mem_accept(const struct hvm_io_handler *handler,
> > +                                const ioreq_t *p)
> >  {
> > -    struct domain *d = current->domain;
> > -    struct hvm_hw_stdvga *s = &d->arch.hvm_domain.stdvga;
> > -    uint64_t start, end, addr = p->addr, count = p->count, size = p->size;
> > -    int buf = 0, rc;
> > -
> > -    if ( unlikely(p->df) )
> > -    {
> > -        start = (addr - (count - 1) * size);
> > -        end = addr + size;
> > -    }
> > -    else
> > -    {
> > -        start = addr;
> > -        end = addr + count * size;
> > -    }
> > -
> > -    if ( (start < VGA_MEM_BASE) || (end > (VGA_MEM_BASE +
> VGA_MEM_SIZE)) )
> > -        return X86EMUL_UNHANDLEABLE;
> > -
> > -    if ( size > 8 )
> > -    {
> > -        gdprintk(XENLOG_WARNING, "invalid mmio size %d\n", (int)p->size);
> > -        return X86EMUL_UNHANDLEABLE;
> > -    }
> > +    struct hvm_hw_stdvga *s = &current->domain-
> >arch.hvm_domain.stdvga;
> >
> >      spin_lock(&s->lock);
> >
> > -    if ( s->stdvga && s->cache )
> > +    if ( !s->stdvga ||
> > +         (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
> > +         (hvm_mmio_last_byte(p) > (VGA_MEM_BASE + VGA_MEM_SIZE)) )
> 
> If "last" means what is says, you need >= here.
> 

True.

  Paul

> Jan