[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 00/20] hw/i386/pc: Split PIIX3 southbridge from i440FX northbridge



Hi Aleksandar,

On 10/27/19 8:44 AM, Aleksandar Markovic wrote:
On Saturday, October 26, 2019, Philippe Mathieu-Daudé <philmd@xxxxxxxxxx <mailto:philmd@xxxxxxxxxx>> wrote:

    Changes since v2 [0]:
    - Use a #define
    - Reword one description
    - Added review tags (thanks all for reviewing!)

    Changes since v1 [1]:
    - Removed patch reintroducing DO_UPCAST() use (thuth)
    - Took various patches out to reduce series (thuth)
    - Added review tags (thanks all for reviewing!)

    $ git backport-diff -u pc_split_i440fx_piix-v2
    Key:
    [----] : patches are identical
    [####] : number of functional differences between
    upstream/downstream patch
    [down] : patch is downstream-only
    The flags [FC] indicate (F)unctional and (C)ontextual differences,
    respectively

    001/20:[----] [--] 'MAINTAINERS: Keep PIIX4 South Bridge separate
    from PC Chipsets'
    002/20:[0004] [FC] 'piix4: Add the Reset Control Register'
    003/20:[0002] [FC] 'piix4: Add an i8259 Interrupt Controller as
    specified in datasheet'
    004/20:[----] [--] 'Revert "irq: introduce qemu_irq_proxy()"'
    005/20:[----] [--] 'piix4: Rename PIIX4 object to piix4-isa'
    006/20:[----] [--] 'piix4: Add an i8257 DMA Controller as specified
    in datasheet'
    007/20:[----] [-C] 'piix4: Add an i8254 PIT Controller as specified
    in datasheet'
    008/20:[0004] [FC] 'piix4: Add a MC146818 RTC Controller as
    specified in datasheet'
    009/20:[----] [--] 'hw/mips/mips_malta: Create IDE hard drive array
    dynamically'
    010/20:[----] [--] 'hw/mips/mips_malta: Extract the PIIX4 creation
    code as piix4_create()'
    011/20:[----] [-C] 'hw/isa/piix4: Move piix4_create() to hw/isa/piix4.c'
    012/20:[----] [--] 'hw/i386: Remove obsolete
    LoadStateHandler::load_state_old handlers'
    013/20:[----] [--] 'hw/pci-host/piix: Extract piix3_create()'
    014/20:[0002] [FC] 'hw/pci-host/piix: Move RCR_IOPORT register
    definition'
    015/20:[----] [--] 'hw/pci-host/piix: Define and use the PIIX IRQ
    Route Control Registers'
    016/20:[----] [-C] 'hw/pci-host/piix: Move i440FX declarations to
    hw/pci-host/i440fx.h'
    017/20:[----] [--] 'hw/pci-host/piix: Fix code style issues'
    018/20:[----] [--] 'hw/pci-host/piix: Extract PIIX3 functions to
    hw/isa/piix3.c'
    019/20:[----] [--] 'hw/pci-host: Rename incorrectly named 'piix' as
    'i440fx''
    020/20:[0004] [FC] 'hw/pci-host/i440fx: Remove the last PIIX3 traces'

    Previous cover:

    This series is a rework of "piix4: cleanup and improvements" [2]
    from Hervé, and my "remove i386/pc dependency: PIIX cleanup" [3].

    Still trying to remove the strong X86/PC dependency 2 years later,
    one step at a time.
    Here we split the PIIX3 southbridge from i440FX northbridge.
    The i440FX northbridge is only used by the PC machine, while the
    PIIX southbridge is also used by the Malta MIPS machine.

    This is also a step forward using KConfig with the Malta board.
    Without this split, it was impossible to compile the Malta without
    pulling various X86 pieces of code.

    The overall design cleanup is not yet perfect, but enough to post
    as a series.

    Now that the PIIX3 code is extracted, the code duplication with the
    PIIX4 chipset is obvious. Not worth improving for now because it
    isn't broken.

    Based-on: <1572097538-18898-1-git-send-email-pbonzini@xxxxxxxxxx
    <mailto:1572097538-18898-1-git-send-email-pbonzini@xxxxxxxxxx>>
    to include:
    mc146818rtc: Allow call object_initialize(MC146818_RTC) instead of
    rtc_init()
    https://mid.mail-archive.com/20191018133547.10936-1-philmd@xxxxxxxxxx 
<https://mid.mail-archive.com/20191018133547.10936-1-philmd@xxxxxxxxxx>

    Since Aleksandar offered me to send the pull request [4] I plan to do
    it once Paolo's pull [5] is merged.


Philippe,

I attempted the other day the integration of v2 of this series into MIPS pull request, but couldn't do it - since another series of yours was already merged, acting on the same code, making rebasing difficult. Now this, v3, series can't be applied since certain patches in some, on surface, unrelated series aren't megred, and v3 assumes they are merged.

If you send a series, it should preferably be based on the latest (current) code base, not on some imagined future state.

I used the 'based-on' tag to refer other series, and patchew succeeded
at applying this series on top of it and build/test it.

Based-on: <1572097538-18898-1-git-send-email-pbonzini@xxxxxxxxxx

Why did you create this such mess with interdependencies of your own multiple series, and just right before softfreeze? :( You should have distributed submitting those series over longer time interval, and absolutely avoid, if possible, this hectic around-softfreeze period. You did the opposite: waited for softfreeze to become close, and sent several interdependant series in matter of days - creating stress without any real technical reason.

This series touches multiple subsystems, so different maintainers had to take the various parts, so I had to split.

Also Peter gave the recommendation on the list to not put more than 20 patches in a series, to make it digestible for review.

In case you, for any reason, can't complete this by softfreeze, I advice you not to rush, and postpone the integration to 5.0.

This series doesn't provide any useful feature, it is a simple cleanup,
posted and reviewed before soft freeze, so we still have 1 week (until
hard freeze) to have it merged, or postpone. No need to stress out for
a cleanup ;)

Regards,

Phil.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.