On 201210 1411, Philippe Mathieu-Daudé wrote:
> On 12/10/20 12:36 PM, Darren Kenny wrote:
> > Hi Alex,
> >
> > On Wednesday, 2020-12-09 at 15:10:54 -05, Alexander Bulekov wrote:
> >> Prior to this patch, the fuzzer found inputs to map PCI device BARs and
> >> enable the device. While it is nice that the fuzzer can do this, it
> >> added significant overhead, since the fuzzer needs to map all the
> >> BARs (regenerating the memory topology), at the start of each input.
> >> With this patch, we do this once, before fuzzing, mitigating some of
> >> this overhead.
> >>
> >> Signed-off-by: Alexander Bulekov <alx...@bu.edu>
> >
> > In general this looks good, I've a small comment/nit below, but nothing
> > serious, so:
> >
> > Reviewed-by: Darren Kenny <darren.ke...@oracle.com>
> >
> >> ---
> >> tests/qtest/fuzz/generic_fuzz.c | 23 +++++++++++++++++++++++
> >> 1 file changed, 23 insertions(+)
> >>
> >> diff --git a/tests/qtest/fuzz/generic_fuzz.c
> >> b/tests/qtest/fuzz/generic_fuzz.c
> >> index 07ad690683..d95093ee53 100644
> >> --- a/tests/qtest/fuzz/generic_fuzz.c
> >> +++ b/tests/qtest/fuzz/generic_fuzz.c
> >> @@ -16,6 +16,7 @@
> >>
> >> #include "hw/core/cpu.h"
> >> #include "tests/qtest/libqos/libqtest.h"
> >> +#include "tests/qtest/libqos/pci-pc.h"
> >> #include "fuzz.h"
> >> #include "fork_fuzz.h"
> >> #include "exec/address-spaces.h"
> >> @@ -762,6 +763,22 @@ static int locate_fuzz_objects(Object *child, void
> >> *opaque)
> >> return 0;
> >> }
> >>
> >> +
> >> +static void pci_enum(gpointer pcidev, gpointer bus)
> >> +{
> >> + PCIDevice *dev = pcidev;
> >> + QPCIDevice *qdev;
> >> +
> >> + qdev = qpci_device_find(bus, dev->devfn);
> >> + g_assert(qdev != NULL);
> >> + for (int i = 0; i < 6; i++) {
> >> + if (dev->io_regions[i].size) {
> >> + qpci_iomap(qdev, i, NULL);
> >> + }
> >> + }
> >> + qpci_device_enable(qdev);
> >> +}
> >> +
> >> static void generic_pre_fuzz(QTestState *s)
> >> {
> >> GHashTableIter iter;
> >> @@ -810,6 +827,12 @@ static void generic_pre_fuzz(QTestState *s)
> >> exit(1);
> >> }
> >>
> >> + QPCIBus *pcibus;
> >
> > NIT: I'm not a huge fan of defining variables in the middle of code,
> > call me old-fashioned if you will, but I tend to prefer them at the
> > top of the function, or block ;)
>
> This is barely tolerated in for(;;) loops.
>
> See commit 7be41675f7c ("configure: Force the C standard to gnu99")
> and QEMU CODING_STYLE.rst:
>
> Declarations
> ============
>
> Mixed declarations (interleaving statements and declarations within
> blocks) are generally not allowed; declarations should be at the
> beginning of blocks.
>
> Every now and then, an exception is made for declarations inside a
> #ifdef or #ifndef block: if the code looks nicer, such declarations can
> be placed at the top of the block even if there are statements above.
> On the other hand, however, it's often best to move that #ifdef/#ifndef
> block to a separate function altogether.
>
> Regards,
>
Sounds good - I'll send out a v2.
Thanks
-Alex
> Phil.
>
