Re: [PATCH kvmtool 08/21] x86/ioport: Refactor trap handlers

Alexandru Elisei
2021-02-12T11:15:28Z

Hi Andre,

On 12/10/20 2:28 PM, Andre Przywara wrote:
> With the planned retirement of the special ioport emulation code, we
> need to provide emulation functions compatible with the MMIO
> prototype.
>
> Adjust the trap handlers to use that new function, and provide shims to
> implement the old ioport interface, for now.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> x86/ioport.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/x86/ioport.c b/x86/ioport.c
> index 8c5c7699..932da20a 100644
> --- a/x86/ioport.c
> +++ b/x86/ioport.c
> @@ -3,8 +3,14 @@
> #include <stdlib.h>
> #include <stdio.h>
>
> +static void dummy_mmio(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len,
> + u8 is_write, void *ptr)
> +{
> +}
> +
> static bool debug_io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> {
> + dummy_mmio(vcpu, port, data, size, true, NULL);
> return 0;
> }
>
> @@ -12,15 +18,23 @@ static struct ioport_operations debug_ops = {
> .io_out = debug_io_out,
> };
>
> -static bool seabios_debug_io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> +static void seabios_debug_mmio(struct kvm_cpu *vcpu, u64 addr, u8 *data,
> + u32 len, u8 is_write, void *ptr)
> {
> char ch;
>
> + if (!is_write)
> + return;
> +
> ch = ioport__read8(data);
>
> putchar(ch);
> +}
>
> - return true;
> +static bool seabios_debug_io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> +{
> + seabios_debug_mmio(vcpu, port, data, size, true, NULL);
> + return 0;
> }
>
> static struct ioport_operations seabios_debug_ops = {
> @@ -29,11 +43,13 @@ static struct ioport_operations seabios_debug_ops = {
>
> static bool dummy_io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> {
> + dummy_mmio(vcpu, port, data, size, false, NULL);
> return true;
> }
>
> static bool dummy_io_out(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> {
> + dummy_mmio(vcpu, port, data, size, true, NULL);
> return true;
> }
>
> @@ -50,13 +66,19 @@ static struct ioport_operations dummy_write_only_ioport_ops = {
> * The "fast A20 gate"
> */
>
> -static bool ps2_control_a_io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> +static void ps2_control_mmio(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len,
> + u8 is_write, void *ptr)
> {
> /*
> * A20 is always enabled.
> */
> - ioport__write8(data, 0x02);
> + if (!is_write)
> + ioport__write8(data, 0x02);
> +}
>
> +static bool ps2_control_a_io_in(struct ioport *ioport, struct kvm_cpu *vcpu, u16 port, void *data, int size)
> +{
> + ps2_control_mmio(vcpu, port, data, size, false, NULL);
> return true;
> }
>

Looks correct to me, if not particularly pretty; thankfully the next patch removes
all of these dummy functions. It compiles, so:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex