Re: [Xen-devel] [PATCH 18/38] arm: implement vpl011 (UART) emulator.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-06-07 at 10:29 +0100, Tim Deegan wrote:
> > +int domain_uart0_init(struct domain *d)
> > +{
> > +    int rc;
> > +    if ( d->domain_id == 0 )
> > +        return 0;
> 
> Why?  There's no equivalent gate on the MMIO handlers.

dom0 has the actual uart mapped at this address, not the emulated one.
Maybe that should be written at the caller instead?

> 
> > +    spin_lock_init(&d->arch.uart0.lock);
> > +    d->arch.uart0.idx = 0;
> > +
> > +    rc = -ENOMEM;
> > +    d->arch.uart0.buf = xzalloc_array(char, VPL011_BUF_SIZE);
> > +    if ( !d->arch.uart0.buf )
> > +        goto out;
> 
> Just return ENOMEM here - the 'rc=foo; goto out' is overkill.

ok.

> > +
> > +    rc = 0;
> > +out:
> > +    return rc;
> > +}
> 
> > +static int uart0_mmio_check(struct vcpu *v, paddr_t addr)
> > +{
> > +    return v->domain->domain_id && addr >= UART0_BASE_ADDRESS && addr < 
> > (UART0_BASE_ADDRESS+65536);
> > +}
> 
> linewrap?

yes, Stefano also suggests a better #define than 65536...

> 
> > +
> > +static int uart0_mmio_read(struct vcpu *v, mmio_info_t *info)
> > +{
> > +    struct hsr_dabt dabt = info->dabt;
> > +    struct cpu_user_regs *regs = guest_cpu_user_regs();
> > +    uint32_t *r = &regs->r0 + dabt.reg;
> > +    int offset = (int)(info->gpa - UART0_BASE_ADDRESS);
> > +
> 
> Need to check dabt.size != 0 here too?

IIRC I was seeing reads of different sizes. To be honest I mostly
tailored this for the specific behaviour of the Linux decompression
code, it didn't really want to write a full & correct UART emulation...

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel