[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/3] ipmi: Use proper struct reference for BT
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/3] ipmi: Use proper struct reference for BT vmstate |
Date: |
Wed, 25 Apr 2018 17:27:50 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
Hi Corey,
On 04/25/2018 12:27 PM, address@hidden wrote:
> From: Corey Minyard <address@hidden>
>
> The vmstate for isa_ipmi_bt was referencing into the bt structure,
> instead create a bt structure separate and use that.
>
> The version 1 of the BT transfer was fairly broken, if a migration
> occured during an IPMI operation, it is likely the migration would
> be corrupted because I misunderstood the VMSTATE_VBUFFER_UINT32()
> handling, I thought it handled transferring the length field,
> too. So I just remove support for that. I doubt anyone is using
> it at this point.
>
> This also removes the transfer of use_irq, since that should come
> from configuration.
>
> Signed-off-by: Corey Minyard <address@hidden>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
It seems you mis-pasted Signed-off-by -> Reviewed-by.
> ---
> hw/ipmi/isa_ipmi_bt.c | 68
> +++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 55 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
> index e946030..8bbb1fa 100644
> --- a/hw/ipmi/isa_ipmi_bt.c
> +++ b/hw/ipmi/isa_ipmi_bt.c
> @@ -22,6 +22,7 @@
> * THE SOFTWARE.
> */
> #include "qemu/osdep.h"
> +#include "qemu/log.h"
> #include "qapi/error.h"
> #include "hw/hw.h"
> #include "hw/ipmi/ipmi.h"
> @@ -450,22 +451,63 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error
> **errp)
> isa_register_ioport(isadev, &iib->bt.io, iib->bt.io_base);
> }
>
> -static const VMStateDescription vmstate_ISAIPMIBTDevice = {
> - .name = TYPE_IPMI_INTERFACE,
> +static int ipmi_bt_vmstate_post_load(void *opaque, int version)
> +{
> + IPMIBT *ib = opaque;
> +
> + /* Make sure all the values are sane. */
> + if (ib->outpos >= MAX_IPMI_MSG_SIZE || ib->outlen >= MAX_IPMI_MSG_SIZE ||
> + ib->outpos >= ib->outlen) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "ipmi:bt: vmstate transfer received bad out values: %d
> %d\n",
> + ib->outpos, ib->outlen);
> + ib->outpos = 0;
> + ib->outlen = 0;
> + }
> +
> + if (ib->inlen >= MAX_IPMI_MSG_SIZE) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "ipmi:bt: vmstate transfer received bad in value:
> %d\n",
> + ib->inlen);
> + ib->inlen = 0;
> + }
> +
> + return 0;
> +}
> +
> +const VMStateDescription vmstate_IPMIBT = {
> + .name = TYPE_IPMI_INTERFACE_PREFIX "bt",
> .version_id = 1,
> .minimum_version_id = 1,
> + .post_load = ipmi_bt_vmstate_post_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_BOOL(obf_irq_set, IPMIBT),
> + VMSTATE_BOOL(atn_irq_set, IPMIBT),
> + VMSTATE_BOOL(irqs_enabled, IPMIBT),
> + VMSTATE_UINT32(outpos, IPMIBT),
> + VMSTATE_UINT32(outlen, IPMIBT),
> + VMSTATE_UINT8_ARRAY(outmsg, IPMIBT, MAX_IPMI_MSG_SIZE),
> + VMSTATE_UINT32(inlen, IPMIBT),
> + VMSTATE_UINT8_ARRAY(inmsg, IPMIBT, MAX_IPMI_MSG_SIZE),
> + VMSTATE_UINT8(control_reg, IPMIBT),
> + VMSTATE_UINT8(mask_reg, IPMIBT),
> + VMSTATE_UINT8(waiting_rsp, IPMIBT),
> + VMSTATE_UINT8(waiting_seq, IPMIBT),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static const VMStateDescription vmstate_ISAIPMIBTDevice = {
> + .name = TYPE_IPMI_INTERFACE_PREFIX "isa-bt",
> + .version_id = 2,
> + .minimum_version_id = 2,
> + /*
> + * Version 1 had messed up the array transfer, it's not even usable
> + * because it used VMSTATE_VBUFFER_UINT32, but it did not transfer
> + * the buffer length, so random things would happen.
> + */
> .fields = (VMStateField[]) {
> - VMSTATE_BOOL(bt.obf_irq_set, ISAIPMIBTDevice),
> - VMSTATE_BOOL(bt.atn_irq_set, ISAIPMIBTDevice),
> - VMSTATE_BOOL(bt.use_irq, ISAIPMIBTDevice),
> - VMSTATE_BOOL(bt.irqs_enabled, ISAIPMIBTDevice),
> - VMSTATE_UINT32(bt.outpos, ISAIPMIBTDevice),
> - VMSTATE_VBUFFER_UINT32(bt.outmsg, ISAIPMIBTDevice, 1, NULL,
> bt.outlen),
> - VMSTATE_VBUFFER_UINT32(bt.inmsg, ISAIPMIBTDevice, 1, NULL, bt.inlen),
> - VMSTATE_UINT8(bt.control_reg, ISAIPMIBTDevice),
> - VMSTATE_UINT8(bt.mask_reg, ISAIPMIBTDevice),
> - VMSTATE_UINT8(bt.waiting_rsp, ISAIPMIBTDevice),
> - VMSTATE_UINT8(bt.waiting_seq, ISAIPMIBTDevice),
> + VMSTATE_STRUCT(bt, ISAIPMIBTDevice, 1, vmstate_IPMIBT, IPMIBT),
> VMSTATE_END_OF_LIST()
> }
> };
>