[kvmtool,v2,13/22] hw/serial: Refactor trap handler

Message ID	20210225005915.26423-14-andre.przywara@arm.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <kvm-owner@kernel.org> From: Andre Przywara <andre.przywara@arm.com> To: Will Deacon <will@kernel.org>, Julien Thierry <julien.thierry.kdev@gmail.com> Cc: Alexandru Elisei <alexandru.elisei@arm.com>, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, Marc Zyngier <maz@kernel.org>, Sami Mujawar <sami.mujawar@arm.com> Subject: [PATCH kvmtool v2 13/22] hw/serial: Refactor trap handler Date: Thu, 25 Feb 2021 00:59:06 +0000 Message-Id: <20210225005915.26423-14-andre.przywara@arm.com> In-Reply-To: <20210225005915.26423-1-andre.przywara@arm.com> References: <20210225005915.26423-1-andre.przywara@arm.com> Precedence: bulk
Series	Unify I/O port and MMIO trap handling \| expand [kvmtool,v2,00/22] Unify I/O port and MMIO trap handling [kvmtool,v2,01/22] ioport: Remove ioport__setup_arch() [kvmtool,v2,02/22] hw/serial: Use device abstraction for FDT generator function [kvmtool,v2,03/22] ioport: Retire .generate_fdt_node functionality [kvmtool,v2,04/22] mmio: Extend handling to include ioport emulation [kvmtool,v2,05/22] hw/i8042: Clean up data types [kvmtool,v2,06/22] hw/i8042: Refactor trap handler [kvmtool,v2,07/22] hw/i8042: Switch to new trap handlers [kvmtool,v2,08/22] x86/ioport: Refactor trap handlers [kvmtool,v2,09/22] x86/ioport: Switch to new trap handlers [kvmtool,v2,10/22] hw/rtc: Refactor trap handlers [kvmtool,v2,11/22] hw/rtc: Switch to new trap handler [kvmtool,v2,12/22] hw/vesa: Switch trap handling to use MMIO handler [kvmtool,v2,13/22] hw/serial: Refactor trap handler [kvmtool,v2,14/22] hw/serial: Switch to new trap handlers [kvmtool,v2,15/22] vfio: Refactor ioport trap handler [kvmtool,v2,16/22] vfio: Switch to new ioport trap handlers [kvmtool,v2,17/22] virtio: Switch trap handling to use MMIO handler [kvmtool,v2,18/22] pci: Switch trap handling to use MMIO handler [kvmtool,v2,19/22] Remove ioport specific routines [kvmtool,v2,20/22] arm: Reorganise and document memory map [kvmtool,v2,21/22] hw/serial: ARM/arm64: Use MMIO at higher addresses [kvmtool,v2,22/22] hw/rtc: ARM/arm64: Use MMIO at higher addresses

Message ID

20210225005915.26423-14-andre.przywara@arm.com (mailing list archive)

State

New, archived

Headers

From: Andre Przywara <andre.przywara@arm.com>
To: Will Deacon <will@kernel.org>,
        Julien Thierry <julien.thierry.kdev@gmail.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>, kvm@vger.kernel.org,
        kvmarm@lists.cs.columbia.edu, Marc Zyngier <maz@kernel.org>,
        Sami Mujawar <sami.mujawar@arm.com>
Subject: [PATCH kvmtool v2 13/22] hw/serial: Refactor trap handler
Date: Thu, 25 Feb 2021 00:59:06 +0000
Message-Id: <20210225005915.26423-14-andre.przywara@arm.com>
In-Reply-To: <20210225005915.26423-1-andre.przywara@arm.com>
References: <20210225005915.26423-1-andre.przywara@arm.com>
Precedence: bulk

Series

Unify I/O port and MMIO trap handling | expand

Commit Message

Andre Przywara Feb. 25, 2021, 12:59 a.m. UTC

With the planned retirement of the special ioport emulation code, we
need to provide an emulation function compatible with the MMIO prototype.

Adjust the trap handler to use that new function, and provide shims to
implement the old ioport interface, for now.

We drop the usage of ioport__read8/write8 entirely, as this would only
be applicable for I/O port accesses, and does nothing for 8-bit wide
accesses anyway.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 hw/serial.c | 93 +++++++++++++++++++++++++++++++++--------------------
 1 file changed, 58 insertions(+), 35 deletions(-)

Comments

Alexandru Elisei March 12, 2021, 11:29 a.m. UTC | #1

Hi Andre,

On 2/25/21 12:59 AM, Andre Przywara wrote:
> With the planned retirement of the special ioport emulation code, we
> need to provide an emulation function compatible with the MMIO prototype.
>
> Adjust the trap handler to use that new function, and provide shims to
> implement the old ioport interface, for now.
>
> We drop the usage of ioport__read8/write8 entirely, as this would only
> be applicable for I/O port accesses, and does nothing for 8-bit wide
> accesses anyway.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  hw/serial.c | 93 +++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 58 insertions(+), 35 deletions(-)
>
> diff --git a/hw/serial.c b/hw/serial.c
> index b0465d99..c495eac1 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -242,36 +242,31 @@ void serial8250__inject_sysrq(struct kvm *kvm, char sysrq)
>  	sysrq_pending = sysrq;
>  }
>  
> -static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
> -			   void *data, int size)
> +static bool serial8250_out(struct serial8250_device *dev, struct kvm_cpu *vcpu,
> +			   u16 offset, u8 data)
>  {
> -	struct serial8250_device *dev = ioport->priv;
> -	u16 offset;
>  	bool ret = true;
> -	char *addr = data;
>  
>  	mutex_lock(&dev->mutex);
>  
> -	offset = port - dev->iobase;
> -
>  	switch (offset) {
>  	case UART_TX:
>  		if (dev->lcr & UART_LCR_DLAB) {
> -			dev->dll = ioport__read8(data);
> +			dev->dll = data;
>  			break;
>  		}
>  
>  		/* Loopback mode */
>  		if (dev->mcr & UART_MCR_LOOP) {
>  			if (dev->rxcnt < FIFO_LEN) {
> -				dev->rxbuf[dev->rxcnt++] = *addr;
> +				dev->rxbuf[dev->rxcnt++] = data;
>  				dev->lsr |= UART_LSR_DR;
>  			}
>  			break;
>  		}
>  
>  		if (dev->txcnt < FIFO_LEN) {
> -			dev->txbuf[dev->txcnt++] = *addr;
> +			dev->txbuf[dev->txcnt++] = data;
>  			dev->lsr &= ~UART_LSR_TEMT;
>  			if (dev->txcnt == FIFO_LEN / 2)
>  				dev->lsr &= ~UART_LSR_THRE;
> @@ -283,18 +278,18 @@ static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
>  		break;
>  	case UART_IER:
>  		if (!(dev->lcr & UART_LCR_DLAB))
> -			dev->ier = ioport__read8(data) & 0x0f;
> +			dev->ier = data & 0x0f;
>  		else
> -			dev->dlm = ioport__read8(data);
> +			dev->dlm = data;
>  		break;
>  	case UART_FCR:
> -		dev->fcr = ioport__read8(data);
> +		dev->fcr = data;
>  		break;
>  	case UART_LCR:
> -		dev->lcr = ioport__read8(data);
> +		dev->lcr = data;
>  		break;
>  	case UART_MCR:
> -		dev->mcr = ioport__read8(data);
> +		dev->mcr = data;
>  		break;
>  	case UART_LSR:
>  		/* Factory test */
> @@ -303,7 +298,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
>  		/* Not used */
>  		break;
>  	case UART_SCR:
> -		dev->scr = ioport__read8(data);
> +		dev->scr = data;
>  		break;
>  	default:
>  		ret = false;
> @@ -317,7 +312,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
>  	return ret;
>  }
>  
> -static void serial8250_rx(struct serial8250_device *dev, void *data)
> +static void serial8250_rx(struct serial8250_device *dev, u8 *data)
>  {
>  	if (dev->rxdone == dev->rxcnt)
>  		return;
> @@ -325,57 +320,54 @@ static void serial8250_rx(struct serial8250_device *dev, void *data)
>  	/* Break issued ? */
>  	if (dev->lsr & UART_LSR_BI) {
>  		dev->lsr &= ~UART_LSR_BI;
> -		ioport__write8(data, 0);
> +		*data = 0;
>  		return;
>  	}
>  
> -	ioport__write8(data, dev->rxbuf[dev->rxdone++]);
> +	*data = dev->rxbuf[dev->rxdone++];
>  	if (dev->rxcnt == dev->rxdone) {
>  		dev->lsr &= ~UART_LSR_DR;
>  		dev->rxcnt = dev->rxdone = 0;
>  	}
>  }
>  
> -static bool serial8250_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> +static bool serial8250_in(struct serial8250_device *dev, struct kvm_cpu *vcpu,
> +			  u16 offset, u8 *data)
>  {
> -	struct serial8250_device *dev = ioport->priv;
> -	u16 offset;
>  	bool ret = true;
>  
>  	mutex_lock(&dev->mutex);
>  
> -	offset = port - dev->iobase;
> -
>  	switch (offset) {
>  	case UART_RX:
>  		if (dev->lcr & UART_LCR_DLAB)
> -			ioport__write8(data, dev->dll);
> +			*data = dev->dll;
>  		else
>  			serial8250_rx(dev, data);
>  		break;
>  	case UART_IER:
>  		if (dev->lcr & UART_LCR_DLAB)
> -			ioport__write8(data, dev->dlm);
> +			*data = dev->dlm;
>  		else
> -			ioport__write8(data, dev->ier);
> +			*data = dev->ier;
>  		break;
>  	case UART_IIR:
> -		ioport__write8(data, dev->iir | UART_IIR_TYPE_BITS);
> +		*data = dev->iir | UART_IIR_TYPE_BITS;
>  		break;
>  	case UART_LCR:
> -		ioport__write8(data, dev->lcr);
> +		*data = dev->lcr;
>  		break;
>  	case UART_MCR:
> -		ioport__write8(data, dev->mcr);
> +		*data = dev->mcr;
>  		break;
>  	case UART_LSR:
> -		ioport__write8(data, dev->lsr);
> +		*data = dev->lsr;
>  		break;
>  	case UART_MSR:
> -		ioport__write8(data, dev->msr);
> +		*data = dev->msr;
>  		break;
>  	case UART_SCR:
> -		ioport__write8(data, dev->scr);
> +		*data = dev->scr;
>  		break;
>  	default:
>  		ret = false;
> @@ -389,6 +381,37 @@ static bool serial8250_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
>  	return ret;
>  }

I'm not sure about replacing ioport_{read,write}8 with pointer dereferencing. They
are functionally equivalent because big-endian and little-endian are about the
order of bytes in memory, not the order of bits, but I don't think the change
makes much sense because:

1. All the other emulated devices use the ioport accessors, even in the case of 8
bit accesses. For example, the RTC device uses ioport accessors and it too is
converted in the end to a MMIO device, just like serial.

2. The patch is about refactoring the emulation callbacks, other (non-trivial)
changes should be a separate patch.

Please let me know if it's something that I'm missing. Other than this change, the
patch looks good.

Thanks,

Alex

>  
> +static void serial8250_mmio(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len,
> +			    u8 is_write, void *ptr)
> +{
> +	struct serial8250_device *dev = ptr;
> +
> +	if (is_write)
> +		serial8250_out(dev, vcpu, addr - dev->iobase, *data);
> +	else
> +		serial8250_in(dev, vcpu, addr - dev->iobase, data);
> +}
> +
> +static bool serial8250_ioport_out(struct ioport *ioport, struct kvm_cpu *vcpu,
> +				  u16 port, void *data, int size)
> +{
> +	struct serial8250_device *dev = ioport->priv;
> +
> +	serial8250_mmio(vcpu, port, data, 1, true, dev);
> +
> +	return true;
> +}
> +
> +static bool serial8250_ioport_in(struct ioport *ioport, struct kvm_cpu *vcpu,
> +				 u16 port, void *data, int size)
> +{
> +	struct serial8250_device *dev = ioport->priv;
> +
> +	serial8250_mmio(vcpu, port, data, 1, false, dev);
> +
> +	return true;
> +}
> +
>  #ifdef CONFIG_HAS_LIBFDT
>  
>  char *fdt_stdout_path = NULL;
> @@ -427,8 +450,8 @@ void serial8250_generate_fdt_node(void *fdt, struct device_header *dev_hdr,
>  #endif
>  
>  static struct ioport_operations serial8250_ops = {
> -	.io_in			= serial8250_in,
> -	.io_out			= serial8250_out,
> +	.io_in			= serial8250_ioport_in,
> +	.io_out			= serial8250_ioport_out,
>  };
>  
>  static int serial8250__device_init(struct kvm *kvm,

Andre Przywara March 15, 2021, 12:26 p.m. UTC | #2

On Fri, 12 Mar 2021 11:29:43 +0000
Alexandru Elisei <alexandru.elisei@arm.com> wrote:

> Hi Andre,
> 
> On 2/25/21 12:59 AM, Andre Przywara wrote:
> > With the planned retirement of the special ioport emulation code, we
> > need to provide an emulation function compatible with the MMIO prototype.
> >
> > Adjust the trap handler to use that new function, and provide shims to
> > implement the old ioport interface, for now.
> >
> > We drop the usage of ioport__read8/write8 entirely, as this would only
> > be applicable for I/O port accesses, and does nothing for 8-bit wide
> > accesses anyway.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  hw/serial.c | 93 +++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 58 insertions(+), 35 deletions(-)
> >
> > diff --git a/hw/serial.c b/hw/serial.c
> > index b0465d99..c495eac1 100644
> > --- a/hw/serial.c
> > +++ b/hw/serial.c
> > @@ -242,36 +242,31 @@ void serial8250__inject_sysrq(struct kvm *kvm, char sysrq)
> >  	sysrq_pending = sysrq;
> >  }
> >  
> > -static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
> > -			   void *data, int size)
> > +static bool serial8250_out(struct serial8250_device *dev, struct kvm_cpu *vcpu,
> > +			   u16 offset, u8 data)
> >  {
> > -	struct serial8250_device *dev = ioport->priv;
> > -	u16 offset;
> >  	bool ret = true;
> > -	char *addr = data;
> >  
> >  	mutex_lock(&dev->mutex);
> >  
> > -	offset = port - dev->iobase;
> > -
> >  	switch (offset) {
> >  	case UART_TX:
> >  		if (dev->lcr & UART_LCR_DLAB) {
> > -			dev->dll = ioport__read8(data);
> > +			dev->dll = data;
> >  			break;
> >  		}
> >  
> >  		/* Loopback mode */
> >  		if (dev->mcr & UART_MCR_LOOP) {
> >  			if (dev->rxcnt < FIFO_LEN) {
> > -				dev->rxbuf[dev->rxcnt++] = *addr;
> > +				dev->rxbuf[dev->rxcnt++] = data;
> >  				dev->lsr |= UART_LSR_DR;
> >  			}
> >  			break;
> >  		}
> >  
> >  		if (dev->txcnt < FIFO_LEN) {
> > -			dev->txbuf[dev->txcnt++] = *addr;
> > +			dev->txbuf[dev->txcnt++] = data;
> >  			dev->lsr &= ~UART_LSR_TEMT;
> >  			if (dev->txcnt == FIFO_LEN / 2)
> >  				dev->lsr &= ~UART_LSR_THRE;
> > @@ -283,18 +278,18 @@ static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
> >  		break;
> >  	case UART_IER:
> >  		if (!(dev->lcr & UART_LCR_DLAB))
> > -			dev->ier = ioport__read8(data) & 0x0f;
> > +			dev->ier = data & 0x0f;
> >  		else
> > -			dev->dlm = ioport__read8(data);
> > +			dev->dlm = data;
> >  		break;
> >  	case UART_FCR:
> > -		dev->fcr = ioport__read8(data);
> > +		dev->fcr = data;
> >  		break;
> >  	case UART_LCR:
> > -		dev->lcr = ioport__read8(data);
> > +		dev->lcr = data;
> >  		break;
> >  	case UART_MCR:
> > -		dev->mcr = ioport__read8(data);
> > +		dev->mcr = data;
> >  		break;
> >  	case UART_LSR:
> >  		/* Factory test */
> > @@ -303,7 +298,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
> >  		/* Not used */
> >  		break;
> >  	case UART_SCR:
> > -		dev->scr = ioport__read8(data);
> > +		dev->scr = data;
> >  		break;
> >  	default:
> >  		ret = false;
> > @@ -317,7 +312,7 @@ static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
> >  	return ret;
> >  }
> >  
> > -static void serial8250_rx(struct serial8250_device *dev, void *data)
> > +static void serial8250_rx(struct serial8250_device *dev, u8 *data)
> >  {
> >  	if (dev->rxdone == dev->rxcnt)
> >  		return;
> > @@ -325,57 +320,54 @@ static void serial8250_rx(struct serial8250_device *dev, void *data)
> >  	/* Break issued ? */
> >  	if (dev->lsr & UART_LSR_BI) {
> >  		dev->lsr &= ~UART_LSR_BI;
> > -		ioport__write8(data, 0);
> > +		*data = 0;
> >  		return;
> >  	}
> >  
> > -	ioport__write8(data, dev->rxbuf[dev->rxdone++]);
> > +	*data = dev->rxbuf[dev->rxdone++];
> >  	if (dev->rxcnt == dev->rxdone) {
> >  		dev->lsr &= ~UART_LSR_DR;
> >  		dev->rxcnt = dev->rxdone = 0;
> >  	}
> >  }
> >  
> > -static bool serial8250_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> > +static bool serial8250_in(struct serial8250_device *dev, struct kvm_cpu *vcpu,
> > +			  u16 offset, u8 *data)
> >  {
> > -	struct serial8250_device *dev = ioport->priv;
> > -	u16 offset;
> >  	bool ret = true;
> >  
> >  	mutex_lock(&dev->mutex);
> >  
> > -	offset = port - dev->iobase;
> > -
> >  	switch (offset) {
> >  	case UART_RX:
> >  		if (dev->lcr & UART_LCR_DLAB)
> > -			ioport__write8(data, dev->dll);
> > +			*data = dev->dll;
> >  		else
> >  			serial8250_rx(dev, data);
> >  		break;
> >  	case UART_IER:
> >  		if (dev->lcr & UART_LCR_DLAB)
> > -			ioport__write8(data, dev->dlm);
> > +			*data = dev->dlm;
> >  		else
> > -			ioport__write8(data, dev->ier);
> > +			*data = dev->ier;
> >  		break;
> >  	case UART_IIR:
> > -		ioport__write8(data, dev->iir | UART_IIR_TYPE_BITS);
> > +		*data = dev->iir | UART_IIR_TYPE_BITS;
> >  		break;
> >  	case UART_LCR:
> > -		ioport__write8(data, dev->lcr);
> > +		*data = dev->lcr;
> >  		break;
> >  	case UART_MCR:
> > -		ioport__write8(data, dev->mcr);
> > +		*data = dev->mcr;
> >  		break;
> >  	case UART_LSR:
> > -		ioport__write8(data, dev->lsr);
> > +		*data = dev->lsr;
> >  		break;
> >  	case UART_MSR:
> > -		ioport__write8(data, dev->msr);
> > +		*data = dev->msr;
> >  		break;
> >  	case UART_SCR:
> > -		ioport__write8(data, dev->scr);
> > +		*data = dev->scr;
> >  		break;
> >  	default:
> >  		ret = false;
> > @@ -389,6 +381,37 @@ static bool serial8250_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
> >  	return ret;
> >  }  
> 
> I'm not sure about replacing ioport_{read,write}8 with pointer dereferencing. They
> are functionally equivalent because big-endian and little-endian are about the
> order of bytes in memory, not the order of bits, but I don't think the change
> makes much sense because:
> 
> 1. All the other emulated devices use the ioport accessors, even in the case of 8
> bit accesses. For example, the RTC device uses ioport accessors and it too is
> converted in the end to a MMIO device, just like serial.
> 
> 2. The patch is about refactoring the emulation callbacks, other (non-trivial)
> changes should be a separate patch.
> 
> Please let me know if it's something that I'm missing. Other than this change, the
> patch looks good.

Well, I can see merits in both approaches: keeping the wrappers and
ditching them. Don't really have a strong opinion, but I think
eventually we should have the endian conversion in one central place
(in kvm__emulate_io()), where we can easily differentiate between port
I/O, MMIO and PCI accesses. Then all devices just need to dereference
pointers.
But this is indeed something for another time, and having wrappers
should not hurt either way, so I will keep them. Makes the patch smaller
anyway.

Cheers,
Andre

> >  
> > +static void serial8250_mmio(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len,
> > +			    u8 is_write, void *ptr)
> > +{
> > +	struct serial8250_device *dev = ptr;
> > +
> > +	if (is_write)
> > +		serial8250_out(dev, vcpu, addr - dev->iobase, *data);
> > +	else
> > +		serial8250_in(dev, vcpu, addr - dev->iobase, data);
> > +}
> > +
> > +static bool serial8250_ioport_out(struct ioport *ioport, struct kvm_cpu *vcpu,
> > +				  u16 port, void *data, int size)
> > +{
> > +	struct serial8250_device *dev = ioport->priv;
> > +
> > +	serial8250_mmio(vcpu, port, data, 1, true, dev);
> > +
> > +	return true;
> > +}
> > +
> > +static bool serial8250_ioport_in(struct ioport *ioport, struct kvm_cpu *vcpu,
> > +				 u16 port, void *data, int size)
> > +{
> > +	struct serial8250_device *dev = ioport->priv;
> > +
> > +	serial8250_mmio(vcpu, port, data, 1, false, dev);
> > +
> > +	return true;
> > +}
> > +
> >  #ifdef CONFIG_HAS_LIBFDT
> >  
> >  char *fdt_stdout_path = NULL;
> > @@ -427,8 +450,8 @@ void serial8250_generate_fdt_node(void *fdt, struct device_header *dev_hdr,
> >  #endif
> >  
> >  static struct ioport_operations serial8250_ops = {
> > -	.io_in			= serial8250_in,
> > -	.io_out			= serial8250_out,
> > +	.io_in			= serial8250_ioport_in,
> > +	.io_out			= serial8250_ioport_out,
> >  };
> >  
> >  static int serial8250__device_init(struct kvm *kvm,

diff --git a/hw/serial.c b/hw/serial.c
index b0465d99..c495eac1 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -242,36 +242,31 @@  void serial8250__inject_sysrq(struct kvm *kvm, char sysrq)
 	sysrq_pending = sysrq;
 }
 
-static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
-			   void *data, int size)
+static bool serial8250_out(struct serial8250_device *dev, struct kvm_cpu *vcpu,
+			   u16 offset, u8 data)
 {
-	struct serial8250_device *dev = ioport->priv;
-	u16 offset;
 	bool ret = true;
-	char *addr = data;
 
 	mutex_lock(&dev->mutex);
 
-	offset = port - dev->iobase;
-
 	switch (offset) {
 	case UART_TX:
 		if (dev->lcr & UART_LCR_DLAB) {
-			dev->dll = ioport__read8(data);
+			dev->dll = data;
 			break;
 		}
 
 		/* Loopback mode */
 		if (dev->mcr & UART_MCR_LOOP) {
 			if (dev->rxcnt < FIFO_LEN) {
-				dev->rxbuf[dev->rxcnt++] = *addr;
+				dev->rxbuf[dev->rxcnt++] = data;
 				dev->lsr |= UART_LSR_DR;
 			}
 			break;
 		}
 
 		if (dev->txcnt < FIFO_LEN) {
-			dev->txbuf[dev->txcnt++] = *addr;
+			dev->txbuf[dev->txcnt++] = data;
 			dev->lsr &= ~UART_LSR_TEMT;
 			if (dev->txcnt == FIFO_LEN / 2)
 				dev->lsr &= ~UART_LSR_THRE;
@@ -283,18 +278,18 @@  static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
 		break;
 	case UART_IER:
 		if (!(dev->lcr & UART_LCR_DLAB))
-			dev->ier = ioport__read8(data) & 0x0f;
+			dev->ier = data & 0x0f;
 		else
-			dev->dlm = ioport__read8(data);
+			dev->dlm = data;
 		break;
 	case UART_FCR:
-		dev->fcr = ioport__read8(data);
+		dev->fcr = data;
 		break;
 	case UART_LCR:
-		dev->lcr = ioport__read8(data);
+		dev->lcr = data;
 		break;
 	case UART_MCR:
-		dev->mcr = ioport__read8(data);
+		dev->mcr = data;
 		break;
 	case UART_LSR:
 		/* Factory test */
@@ -303,7 +298,7 @@  static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
 		/* Not used */
 		break;
 	case UART_SCR:
-		dev->scr = ioport__read8(data);
+		dev->scr = data;
 		break;
 	default:
 		ret = false;
@@ -317,7 +312,7 @@  static bool serial8250_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port
 	return ret;
 }
 
-static void serial8250_rx(struct serial8250_device *dev, void *data)
+static void serial8250_rx(struct serial8250_device *dev, u8 *data)
 {
 	if (dev->rxdone == dev->rxcnt)
 		return;
@@ -325,57 +320,54 @@  static void serial8250_rx(struct serial8250_device *dev, void *data)
 	/* Break issued ? */
 	if (dev->lsr & UART_LSR_BI) {
 		dev->lsr &= ~UART_LSR_BI;
-		ioport__write8(data, 0);
+		*data = 0;
 		return;
 	}
 
-	ioport__write8(data, dev->rxbuf[dev->rxdone++]);
+	*data = dev->rxbuf[dev->rxdone++];
 	if (dev->rxcnt == dev->rxdone) {
 		dev->lsr &= ~UART_LSR_DR;
 		dev->rxcnt = dev->rxdone = 0;
 	}
 }
 
-static bool serial8250_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
+static bool serial8250_in(struct serial8250_device *dev, struct kvm_cpu *vcpu,
+			  u16 offset, u8 *data)
 {
-	struct serial8250_device *dev = ioport->priv;
-	u16 offset;
 	bool ret = true;
 
 	mutex_lock(&dev->mutex);
 
-	offset = port - dev->iobase;
-
 	switch (offset) {
 	case UART_RX:
 		if (dev->lcr & UART_LCR_DLAB)
-			ioport__write8(data, dev->dll);
+			*data = dev->dll;
 		else
 			serial8250_rx(dev, data);
 		break;
 	case UART_IER:
 		if (dev->lcr & UART_LCR_DLAB)
-			ioport__write8(data, dev->dlm);
+			*data = dev->dlm;
 		else
-			ioport__write8(data, dev->ier);
+			*data = dev->ier;
 		break;
 	case UART_IIR:
-		ioport__write8(data, dev->iir | UART_IIR_TYPE_BITS);
+		*data = dev->iir | UART_IIR_TYPE_BITS;
 		break;
 	case UART_LCR:
-		ioport__write8(data, dev->lcr);
+		*data = dev->lcr;
 		break;
 	case UART_MCR:
-		ioport__write8(data, dev->mcr);
+		*data = dev->mcr;
 		break;
 	case UART_LSR:
-		ioport__write8(data, dev->lsr);
+		*data = dev->lsr;
 		break;
 	case UART_MSR:
-		ioport__write8(data, dev->msr);
+		*data = dev->msr;
 		break;
 	case UART_SCR:
-		ioport__write8(data, dev->scr);
+		*data = dev->scr;
 		break;
 	default:
 		ret = false;
@@ -389,6 +381,37 @@  static bool serial8250_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port,
 	return ret;
 }
 
+static void serial8250_mmio(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len,
+			    u8 is_write, void *ptr)
+{
+	struct serial8250_device *dev = ptr;
+
+	if (is_write)
+		serial8250_out(dev, vcpu, addr - dev->iobase, *data);
+	else
+		serial8250_in(dev, vcpu, addr - dev->iobase, data);
+}
+
+static bool serial8250_ioport_out(struct ioport *ioport, struct kvm_cpu *vcpu,
+				  u16 port, void *data, int size)
+{
+	struct serial8250_device *dev = ioport->priv;
+
+	serial8250_mmio(vcpu, port, data, 1, true, dev);
+
+	return true;
+}
+
+static bool serial8250_ioport_in(struct ioport *ioport, struct kvm_cpu *vcpu,
+				 u16 port, void *data, int size)
+{
+	struct serial8250_device *dev = ioport->priv;
+
+	serial8250_mmio(vcpu, port, data, 1, false, dev);
+
+	return true;
+}
+
 #ifdef CONFIG_HAS_LIBFDT
 
 char *fdt_stdout_path = NULL;
@@ -427,8 +450,8 @@  void serial8250_generate_fdt_node(void *fdt, struct device_header *dev_hdr,
 #endif
 
 static struct ioport_operations serial8250_ops = {
-	.io_in			= serial8250_in,
-	.io_out			= serial8250_out,
+	.io_in			= serial8250_ioport_in,
+	.io_out			= serial8250_ioport_out,
 };
 
 static int serial8250__device_init(struct kvm *kvm,

[kvmtool,v2,13/22] hw/serial: Refactor trap handler

Commit Message

Comments

Patch