]>
Commit | Line | Data |
---|---|---|
3ba2ce33 AD |
1 | From patchwork Thu Feb 18 19:31:00 2016 |
2 | Content-Type: text/plain; charset="utf-8" | |
3 | MIME-Version: 1.0 | |
4 | Content-Transfer-Encoding: 8bit | |
5 | Subject: fw_cfg: unbreak migration compatibility for 2.4 and earlier machines | |
6 | From: Laszlo Ersek <lersek@redhat.com> | |
7 | X-Patchwork-Id: 584876 | |
8 | Message-Id: <1455823860-22268-1-git-send-email-lersek@redhat.com> | |
9 | To: qemu-devel@nongnu.org | |
10 | Cc: =?UTF-8?q?Marc=20Mar=ED?= <markmb@redhat.com>, | |
11 | Gerd Hoffmann <kraxel@redhat.com>, | |
12 | Alexandre DERUMIER <aderumier@odiso.com>, qemu-stable@nongnu.org | |
13 | Date: Thu, 18 Feb 2016 20:31:00 +0100 | |
14 | ||
15 | When I reviewed Marc's fw_cfg DMA patches, I completely missed that the | |
16 | way we set dma_enabled would break migration. | |
17 | ||
18 | Gerd explained the right way (see reference below): dma_enabled should be | |
19 | set to true by default, and only true->false transitions should be | |
20 | possible: | |
21 | ||
22 | - when the user requests that with | |
23 | ||
24 | -global fw_cfg_mem.dma_enabled=off | |
25 | ||
26 | or | |
27 | ||
28 | -global fw_cfg_io.dma_enabled=off | |
29 | ||
30 | as appropriate for the platform, | |
31 | ||
32 | - when HW_COMPAT_2_4 dictates it, | |
33 | ||
34 | - when board code initializes fw_cfg without requesting DMA support. | |
35 | ||
36 | Cc: Marc MarĂ <markmb@redhat.com> | |
37 | Cc: Gerd Hoffmann <kraxel@redhat.com> | |
38 | Cc: Alexandre DERUMIER <aderumier@odiso.com> | |
39 | Cc: qemu-stable@nongnu.org | |
40 | Ref: http://thread.gmane.org/gmane.comp.emulators.qemu/390272/focus=391042 | |
41 | Ref: https://bugs.launchpad.net/qemu/+bug/1536487 | |
42 | Suggested-by: Gerd Hoffmann <kraxel@redhat.com> | |
43 | Signed-off-by: Laszlo Ersek <lersek@redhat.com> | |
44 | --- | |
45 | ||
46 | Notes: | |
47 | Tested the following cases with gdb, using qemu-system-x86_64, setting a | |
48 | breakpoint on (s->dma_enabled) in fw_cfg_init_io_dma(): | |
49 | ||
50 | * no special params (DMA enabled) | |
51 | * -global fw_cfg_io.dma_enabled=off (DMA disabled) | |
52 | * -M pc-i440fx-2.4 (DMA disabled), similarly with 2.3 and Q35 too | |
53 | ||
54 | Also tested the memory mapped case in practice, using | |
55 | qemu-system-aarch64 -M virt, -kernel / -initrd / -append, with guest | |
56 | UEFI: | |
57 | * no special params (DMA enabled) | |
58 | * -global fw_cfg_mem.dma_enabled=off (DMA disabled) | |
59 | ||
60 | Not tested: | |
61 | * actual migration | |
62 | * when board code doesn't request DMA support | |
63 | ||
64 | Testing feedback from people who use migration would be nice. | |
65 | ||
66 | include/hw/compat.h | 8 ++++++++ | |
67 | hw/nvram/fw_cfg.c | 20 ++++++++++++-------- | |
68 | 2 files changed, 20 insertions(+), 8 deletions(-) | |
69 | ||
70 | diff --git a/include/hw/compat.h b/include/hw/compat.h | |
71 | index 2ebe739fcb5c..a5dbbf8984b1 100644 | |
72 | index d0b1c4f..b7973db 100644 | |
73 | --- a/include/hw/compat.h | |
74 | +++ b/include/hw/compat.h | |
75 | @@ -18,6 +18,14 @@ | |
76 | .driver = "virtio-pci",\ | |
77 | .property = "migrate-extra",\ | |
78 | .value = "off",\ | |
79 | + },{\ | |
80 | + .driver = "fw_cfg_mem",\ | |
81 | + .property = "dma_enabled",\ | |
82 | + .value = "off",\ | |
83 | + },{\ | |
84 | + .driver = "fw_cfg_io",\ | |
85 | + .property = "dma_enabled",\ | |
86 | + .value = "off",\ | |
87 | }, | |
88 | ||
89 | #define HW_COMPAT_2_3 \ | |
90 | diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c | |
91 | index 79c5742b3362..f3acb47bd4dc 100644 | |
92 | --- a/hw/nvram/fw_cfg.c | |
93 | +++ b/hw/nvram/fw_cfg.c | |
94 | @@ -778,17 +778,19 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, | |
95 | DeviceState *dev; | |
96 | FWCfgState *s; | |
97 | uint32_t version = FW_CFG_VERSION; | |
98 | - bool dma_enabled = dma_iobase && dma_as; | |
99 | + bool dma_requested = dma_iobase && dma_as; | |
100 | ||
101 | dev = qdev_create(NULL, TYPE_FW_CFG_IO); | |
102 | qdev_prop_set_uint32(dev, "iobase", iobase); | |
103 | qdev_prop_set_uint32(dev, "dma_iobase", dma_iobase); | |
104 | - qdev_prop_set_bit(dev, "dma_enabled", dma_enabled); | |
105 | + if (!dma_requested) { | |
106 | + qdev_prop_set_bit(dev, "dma_enabled", false); | |
107 | + } | |
108 | ||
109 | fw_cfg_init1(dev); | |
110 | s = FW_CFG(dev); | |
111 | ||
112 | - if (dma_enabled) { | |
113 | + if (s->dma_enabled) { | |
114 | /* 64 bits for the address field */ | |
115 | s->dma_as = dma_as; | |
116 | s->dma_addr = 0; | |
117 | @@ -814,11 +816,13 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, | |
118 | SysBusDevice *sbd; | |
119 | FWCfgState *s; | |
120 | uint32_t version = FW_CFG_VERSION; | |
121 | - bool dma_enabled = dma_addr && dma_as; | |
122 | + bool dma_requested = dma_addr && dma_as; | |
123 | ||
124 | dev = qdev_create(NULL, TYPE_FW_CFG_MEM); | |
125 | qdev_prop_set_uint32(dev, "data_width", data_width); | |
126 | - qdev_prop_set_bit(dev, "dma_enabled", dma_enabled); | |
127 | + if (!dma_requested) { | |
128 | + qdev_prop_set_bit(dev, "dma_enabled", false); | |
129 | + } | |
130 | ||
131 | fw_cfg_init1(dev); | |
132 | ||
133 | @@ -828,7 +832,7 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr, | |
134 | ||
135 | s = FW_CFG(dev); | |
136 | ||
137 | - if (dma_enabled) { | |
138 | + if (s->dma_enabled) { | |
139 | s->dma_as = dma_as; | |
140 | s->dma_addr = 0; | |
141 | sysbus_mmio_map(sbd, 2, dma_addr); | |
142 | @@ -873,7 +877,7 @@ static Property fw_cfg_io_properties[] = { | |
143 | DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1), | |
144 | DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1), | |
145 | DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled, | |
146 | - false), | |
147 | + true), | |
148 | DEFINE_PROP_END_OF_LIST(), | |
149 | }; | |
150 | ||
151 | @@ -913,7 +917,7 @@ static const TypeInfo fw_cfg_io_info = { | |
152 | static Property fw_cfg_mem_properties[] = { | |
153 | DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1), | |
154 | DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled, | |
155 | - false), | |
156 | + true), | |
157 | DEFINE_PROP_END_OF_LIST(), | |
158 | }; | |
159 |