On 2/19/22 02:12, BALATON Zoltan wrote:
On Fri, 18 Feb 2022, Liav Albani wrote:
This is a preparation before implementing another PCI IDE controller
that relies on these functions, so these can be shared between both
implementations.
Signed-off-by: Liav Albani <liav...@gmail.com>
---
hw/ide/bmdma.c | 84 ++++++++++++++++++++++++++++++++++++++++++
hw/ide/meson.build | 2 +-
hw/ide/piix.c | 51 ++-----------------------
include/hw/ide/bmdma.h | 34 +++++++++++++++++
4 files changed, 122 insertions(+), 49 deletions(-)
create mode 100644 hw/ide/bmdma.c
create mode 100644 include/hw/ide/bmdma.h
diff --git a/hw/ide/bmdma.c b/hw/ide/bmdma.c
new file mode 100644
index 0000000000..d12ed730dd
--- /dev/null
+++ b/hw/ide/bmdma.c
@@ -0,0 +1,84 @@
+/*
+ * QEMU IDE Emulation: Intel PCI Bus master IDE support for PIIX3/4
and ICH6/7.
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * Permission is hereby granted, free of charge, to any person
obtaining a copy
+ * of this software and associated documentation files (the
"Software"), to deal
+ * in the Software without restriction, including without limitation
the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense,
and/or sell
+ * copies of the Software, and to permit persons to whom the
Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/dma.h"
+
+#include "hw/ide/bmdma.h"
+#include "hw/ide/pci.h"
+#include "trace.h"
Are you sure you need all these includes? I haven't checked but I
think there may be some unneeded ones here. On the other hand I'm not
sure putting these in a new file is worth it. There are already some
bmdma_* functions in hw/ide/pci.c which are declared in
include/hw/ide/pci.h so you could just move these there too then no
new file, new header nor changes to meson.build is needed in this patch..
Good question, probably not. I'll try to build without them, adding only
what seems necessary to complete the build. Also, I think adding those
functions to hw/ide/pci.c is a very good idea as it will make the patch
smaller and it also makes total sense to me - Bus Master capabilities
only appear on PCI IDE controllers and not on the ISA IDE controller
(AFAIK).
It will happen in v3 :)
+
+uint64_t intel_ide_bmdma_read(void *opaque, hwaddr addr, unsigned size)
As I said before these aren't intel specific as the same functions
also appear in other ide controllers such as via.c so maybe a better
name would be bmdma_default_read and bmdma_default_write or somehting
similar so these can be also reused by other non-intel devices too.
Right, I see now that via.c uses the exact same functions... I'll change
it in v3. The names bmdma_default_read and bmdma_default_write seem like
a good choice to me.
Regards,
BALATON Zoltan
Thanks for the review!