The register index is currently printed and this is confusing.
Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <pdel@fb.com>
Message-Id: <20211005052604.1674891-3-pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
This model implements enough behaviour to do basic functionality tests
such as device initialisation and read out of dummy sample values. The
sample value generation strategy is similar to the STM ADC already in
the tree.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
[clg : support for multiple engines (AST2600) ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
[pdel : refactored engine register struct fields to regs[] array field]
[pdel : added guest-error checking for upper-8 channel regs in AST2600]
[pdel : allow 16-bit reads of the channel data registers]
Signed-off-by: Peter Delevoryas <pdel@fb.com>
Message-Id: <20211005052604.1674891-2-pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The gpio array is declared as a dense array:
qemu_irq gpios[ASPEED_GPIO_NR_PINS];
(AST2500 has 228, AST2400 has 216, AST2600 has 208)
However, this array is used like a matrix of GPIO sets
(e.g. gpio[NR_SETS][NR_PINS_PER_SET] = gpio[8][32])
size_t offset = set * GPIOS_PER_SET + gpio;
qemu_set_irq(s->gpios[offset], !!(new & mask));
This can result in an out-of-bounds access to "s->gpios" because the
gpio sets do _not_ have the same length. Some of the groups (e.g.
GPIOAB) only have 4 pins. 228 != 8 * 32 == 256.
To fix this, I converted the gpio array from dense to sparse, to that
match both the hardware layout and this existing indexing code.
Fixes: 4b7f956862 ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and AST2500")
Signed-off-by: Peter Delevoryas <pdel@fb.com>
Message-Id: <20211008033501.934729-2-pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Some of the pin declarations in the Aspeed GPIO module were incorrect,
probably because of confusion over which bits in the input and output
uint32_t's correspond to which groups in the label array. Since the
uint32_t literals are in big endian, it's sort of the opposite of what
would be intuitive. The least significant bit in ast2500_set_props[6]
corresponds to GPIOY0, not GPIOAB7.
GPIOxx indicates input and output capabilities, GPIxx indicates only
input, GPOxx indicates only output.
AST2500:
- Previously had GPIW0..GPIW7 and GPIX0..GPIX7, that's correct.
- Previously had GPIOY0..GPIOY3, should have been GPIOY0..GPIOY7.
- Previously had GPIOAB0..GPIOAB3 and GPIAB4..GPIAB7, should only have
been GPIOAB0..GPIOAB3.
AST2600:
- GPIOT0..GPIOT7 should have been GPIT0..GPIT7.
- GPIOU0..GPIOU7 should have been GPIU0..GPIU7.
- GPIW0..GPIW7 should have been GPIOW0..GPIOW7.
- GPIOY0..GPIOY7 and GPIOZ0...GPIOZ7 were disabled.
Fixes: 4b7f956862 ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and AST2500")
Fixes: 36d737ee82 ("hw/gpio: Add in AST2600 specific implementation")
Signed-off-by: Peter Delevoryas <pdel@fb.com>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
Message-Id: <20210928032456.3192603-2-pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Introduce an AspeedI2CBus SysBusDevice model and attach the associated
memory region and IRQ to the newly instantiated objects.
Before this change, the I2C bus IRQs were all attached to the
SysBusDevice model of the I2C controller. Adapt the AST2600 SoC
realize routine to take into account this change.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The AST2400 SPI controller has a transitional HW interface and it
stores the address width currently in use in a different register than
all the other SMC controllers. It needs special handling when working
in 4B mode.
Make it clear through a class handler. This also removes another use
of the segments array.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
This simplifies the reset handler and has the benefit to remove some
"bad" use of the segments array as an identifier of the controller model.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
AspeedSMCFlash is a small structure representing the AHB memory window
through which the contents of a flash device can be accessed with MMIOs.
Introduce an AspeedSMCFlash SysBusDevice model and attach the associated
memory region to the newly instantiated objects.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
'cs' is a more appropriate name to index SPI flash devices.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
AspeedSMCFlash::size is only used to compute the initial size of the
boot_rom region. Not very useful, so directly call memory_region_size()
instead.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
There is no need to keep a reference of the flash qdev in the AspeedSMCFlash
state: the SPI bus takes ownership and will release its resources. Remove
AspeedSMCFlash::flash.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The characteristics of the Aspeed controllers are described in a
AspeedSMCController structure which is redundant with the
AspeedSMCClass. Move all attributes under the class and adapt the code
to use class attributes instead.
This is a large change but it is functionally equivalent.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
There is no real reason to use this name. It's simply nice to have in
the monitor output but it's a burden for the following patch which
removes the AspeedSMCController structure describing the controller.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
It unifies the errors reported by the Aspeed SMC model and also
removes some use of ctrl->name which will help us for the next
patches.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
The Aspeed SoCs have a dual boot function for firmware fail-over
recovery. The system auto-reboots from the second flash if the main
flash does not boot successfully within a certain amount of time. This
function is called alternate boot (ABR) in the FMC controllers.
On AST2400/AST2500, ABR is enabled by hardware strapping in SCU70 to
enable the 2nd watchdog timer, on AST2600, through register SCU510.
If the boot on the the main flash succeeds, the firmware should
disable the 2nd watchdog timer. If not, the BMC is reset and the CE0
and CE1 mappings are swapped to restart the BMC from the 2nd flash.
On the AST2600, the ABR registers controlling the 2nd watchdog timer
were moved from the watchdog register to the FMC controller and the
FMC model should be able to control WDT2 through its own register set.
This requires more work. For now, add dummy read/write handlers to let
the FW disable the 2nd watchdog without error.
Reviewed-by: Peter Delevoryas <pdel@fb.com>
Reported-by: Peter Delevoryas <pdel@fb.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
macfb: fixes for booting MacOS
-----BEGIN PGP SIGNATURE-----
iQJGBAABCAAwFiEEzS913cjjpNwuT1Fz8ww4vT8vvjwFAmFgLywSHGxhdXJlbnRA
dml2aWVyLmV1AAoJEPMMOL0/L748oVMP/0P0PaJzy8bPgeR9dMM+1j8tH//07JEM
K3B+oxs6lYfR3bWSaDtkiVdo+mu1E4eUXNFwCqLrI0U6FRZIyw+1AoOa2nWP1JjN
2resEGQVssBPXU6UJKQVPfRgG9jiVi0qmg9VL62fqwvuH+ek187ioWrgicW26mER
KJcnl+K1VlZRyCS6SaY1zaLjTl+jKie7HC8fLpMhYrcwG0B+GT7c+0yWt4hL+ZdO
EF1VTMZHY03jx2BM/QLWJoxq35ut/7p1rK2tY0uztwCfxsFlh/29iztmcl9ZveAr
ZgA8URpm1cTUfR79p6GciISH27URvS+djxsFUfdk9+52IhVtKSqJSSwkRGkyocvQ
kDre8NqgolM8bbn5R4z7yz+r7FA6Foi8bx0HMaMjo8NhiT2f4uJHb/3aHq0nV2/s
TfYpDR81BPHciuzv36JI1MF/UF0vM6nQo9XL/0rWzfkH7NVLBPzyoHONI7MF4G9V
QtDx/668Q3UZIQ0LBTVgxmSieQdGYoNeKGEO3rWZskkBUW0x8AoamU7SUtPoUl2R
K22nRlg2Cfl+QnW5OEiGQCRXSBWEsFzamf4+QOT+kgK43/bbLSmqwJNsJToRQA4i
3xdH03NG+u1sfCIM/WWVk6B2p86BqHvo5+pwmdFBMp4SeYUFlXDFHzQwljcwn4lp
ZjMWoS5fd6wE
=BRg1
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/vivier-m68k/tags/m68k-next-pull-request' into staging
Pull request q800 20211008
macfb: fixes for booting MacOS
# gpg: Signature made Fri 08 Oct 2021 04:44:44 AM PDT
# gpg: using RSA key CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C
# gpg: issuer "laurent@vivier.eu"
# gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full]
# gpg: aka "Laurent Vivier <laurent@vivier.eu>" [full]
# gpg: aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full]
* remotes/vivier-m68k/tags/m68k-next-pull-request:
q800: wire macfb IRQ to separate video interrupt on VIA2
macfb: add vertical blank interrupt
macfb: fix 24-bit RGB pixel encoding
macfb: fix up 1-bit pixel encoding
macfb: add common monitor modes supported by the MacOS toolbox ROM
macfb: add qdev property to specify display type
macfb: implement mode sense to allow display type to be detected
macfb: add trace events for reading and writing the control registers
macfb: use memory_region_init_ram() in macfb_common_realize() for the framebuffer
macfb: fix overflow of color_palette array
macfb: fix invalid object reference in macfb_common_realize()
macfb: update macfb.c to use the Error API best practices
macfb: handle errors that occur during realize
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Whilst the in-built Quadra 800 framebuffer exists within the Nubus address
space for slot 9, it has its own dedicated interrupt on VIA2. Force the
macfb device to occupy slot 9 in the q800 machine and wire its IRQ to the
separate video interrupt since this is what is expected by the MacOS
interrupt handler.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-14-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The MacOS driver expects a 60.15Hz vertical blank interrupt to be generated by
the framebuffer which in turn schedules the mouse driver via the Vertical Retrace
Manager.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-13-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
According to Apple Technical Note HW26: "Macintosh Quadra Built-In Video" the
in-built framebuffer encodes each 24-bit pixel into 4 bytes. Adjust the 24-bit
RGB pixel encoding accordingly which agrees with the encoding expected by MacOS
when changing into 24-bit colour mode.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-12-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The MacOS driver expects the RGB values for the pixel to be in entries 0 and 1
of the colour palette.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-11-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The monitor modes table is found by experimenting with the Monitors Control
Panel in MacOS and analysing the reads/writes. From this it can be found that
the mode is controlled by writes to the DAFB_MODE_CTRL1 and DAFB_MODE_CTRL2
registers.
Implement the first block of DAFB registers as a register array including the
existing sense register, the newly discovered control registers above, and also
the DAFB_MODE_VADDR1 and DAFB_MODE_VADDR2 registers which are used by NetBSD to
determine the current video mode.
These experiments also show that the offset of the start of video RAM and the
stride can change depending upon the monitor mode, so update macfb_draw_graphic()
and both the BI_MAC_VADDR and BI_MAC_VROW bootinfo for the q800 machine
accordingly.
Finally update macfb_common_realize() so that only the resolution and depth
supported by the display type can be specified on the command line, and add an
error hint showing the list of supported resolutions and depths if the user tries
to specify an invalid display mode.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-10-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Since the available resolutions and colour depths are determined by the attached
display type, add a qdev property to allow the display type to be specified.
The main resolutions of interest are high resolution 1152x870 with 8-bit colour
and SVGA resolution up to 800x600 with 24-bit colour so update the q800 machine
to allow high resolution mode if specified and otherwise fall back to SVGA.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-9-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The MacOS toolbox ROM uses the monitor sense to detect the display type and then
offer a fixed set of resolutions and colour depths accordingly. Implement the
monitor sense using information found in Apple Technical Note HW26: "Macintosh
Quadra Built-In Video" along with some local experiments.
Since the default configuration is 640 x 480 with 8-bit colour then hardcode
the sense register to return MACFB_DISPLAY_VGA for now.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-8-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Currently macfb_common_realize() defines the framebuffer RAM memory region as
being non-migrateable but then immediately registers it for migration. Replace
memory_region_init_ram_nomigrate() with memory_region_init_ram() which is clearer
and does exactly the same thing.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-6-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The palette_current index counter has a maximum size of 256 * 3 to cover a full
color palette of 256 RGB entries. Linux assumes that the palette_current index
wraps back around to zero after writing 256 RGB entries so ensure that
palette_current is reset at this point to prevent data corruption within
MacfbState.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-5-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
During realize memory_region_init_ram_nomigrate() is used to initialise the RAM
memory region used for the framebuffer but the owner object reference is
incorrect since MacFbState is a typedef and not a QOM type.
Change the memory region owner to be the corresponding DeviceState to fix the
issue and prevent random crashes during macfb_common_realize().
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: 8ac919a065 ("hw/m68k: add Nubus macfb video card")
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-4-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
As per the current Error API best practices, change macfb_commom_realize() to return
a boolean indicating success to reduce errp boiler-plate handling code. Note that
memory_region_init_ram_nomigrate() is also updated to use &error_abort to indicate
a non-recoverable error, matching the behaviour recommended after similar
discussions on memory API failures for the recent nubus changes.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-3-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Make sure any errors that occur within the macfb realize chain are detected
and handled correctly to prevent crashes and to ensure that error messages are
reported back to the user.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20211007221253.29024-2-mark.cave-ayland@ilande.co.uk>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
v2: add small fix by Stefano, Hanna's series fixed
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEEi5wmzbL9FHyIDoahVh8kwfGfefsFAmFfEVMACgkQVh8kwfGf
eftAVA//WLtOaiVYPSjEl5EK80kry39VknZkQyeYUzyV7JNr/FRMlbJaIF2sOjH5
KPRpfwBiuijOc8R0s34HY0BpyweRd1rbypHblZkO7EO4XwHx1FLF5kNHF6yV7wPL
c9W564sZpc6Z96wSMgC4Is9QHJ6JbO4TJNJsG8v/PHEqGQV/yCYkgBox4loJckww
uSAZ7l63IWA8uPSq/rOu34bREKN9s0kHkvFq0JNWk2HtOBLDiRDUYmbSfdjfT4jz
np7ojKiffcAJED9JA28Zo2Y+MSId+FyoO4lbt+deMNzIHboy2oVlHouoHHprr61x
dIO7Qt1IoMk5IBIfkPRYkReMwxxSVKuIJcWm8Qqtkcg2X0g5ayNUmHwpBMd50h2z
XPjrr0YdOixhxMHoBnqlkPlWU0Y/B+YJIQ+mjqp+vRNkk94NoXhsXnCod1ajkgWO
zjc/dztew7HvNStJaMM0rnEjanLhzFZKtlMO4WwZHQp2yZG2AINkPStswo2f3AmL
FI+2By/UhFKm3BEemf0wYWDPWrPHU+BOiu16KjSKeS0GA9t7GXBUDRxNYPhUheXJ
eJKIpNsGbseNxKrAbLyRhAB75Fa/ReZqqybmEcLyal/ball3R/cNF3gaMHeX0o1n
HTGIAF5JOAXNGApS5TilkXPZ7jHFOVPh/Fi6/16/08tcgxjVfro=
=TVTu
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/vsementsov/tags/pull-jobs-2021-10-07-v2' into staging
mirror: Handle errors after READY cancel
v2: add small fix by Stefano, Hanna's series fixed
# gpg: Signature made Thu 07 Oct 2021 08:25:07 AM PDT
# gpg: using RSA key 8B9C26CDB2FD147C880E86A1561F24C1F19F79FB
# gpg: Good signature from "Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 8B9C 26CD B2FD 147C 880E 86A1 561F 24C1 F19F 79FB
* remotes/vsementsov/tags/pull-jobs-2021-10-07-v2:
iotests: Add mirror-ready-cancel-error test
mirror: Do not clear .cancelled
mirror: Stop active mirroring after force-cancel
mirror: Check job_is_cancelled() earlier
mirror: Use job_is_cancelled()
job: Add job_cancel_requested()
job: Do not soft-cancel after a job is done
jobs: Give Job.force_cancel more meaning
job: @force parameter for job_cancel_sync()
job: Force-cancel jobs in a failed transaction
mirror: Drop s->synced
mirror: Keep s->synced on error
job: Context changes in job_completed_txn_abort()
block/aio_task: assert `max_busy_tasks` is greater than 0
block/backup: avoid integer overflow of `max-workers`
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Commit 0445409d74 ("iothread: generalize
iothread_set_param/iothread_get_param") moved common code to set and
get IOThread parameters in two new functions.
These functions are called inside callbacks, so we don't need to use an
opaque pointer. Let's replace `void *opaque` parameter with
`IOThreadParamInfo *info`.
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-3-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
a new parameter (aio-max-batch) to IOThread and used PollParamInfo
structure to handle it.
Since it is not a parameter of the polling mechanism, we rename the
structure to a more generic IOThreadParamInfo.
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-2-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
- Add Zb[abcs] instruction support
- Remove RVB support
- Bug fix of setting mstatus_hs.[SD|FS] bits
- Mark some UART devices as 'input'
- QOMify PolarFire MMUART
- Fixes for sifive PDMA
- Mark shakti_c as not user creatable
-----BEGIN PGP SIGNATURE-----
iQEzBAABCAAdFiEE9sSsRtSTSGjTuM6PIeENKd+XcFQFAmFelu0ACgkQIeENKd+X
cFQ1vQf/a0QNMv/q9shDX8IzX6zPKMHpdhgqqfqsdL3bPqjhXBbpZuSQOlq2UsPv
6LwvEqvMq5Il8UHrFGaY87d08NWY+QMXNJml2oe8k5tU80DIR3Iv91Kz6RnGLsR9
iaEFbYZNWax4lIFnW5h543EU4szOuNO+U+egJjvjQARIodbRjsfBMQJzlUHQ7Oqr
2n44165IDUyah3QDXk25SSH1RXoeD1OjrbJ/NLFIY6LE7x9NJZm5GDQr3fIi3E5j
CBBG116huejif9NmaI+xefzi696iZWrbJvhCD9nIlbYnwq4ENgjf4GPb7TEzVPWc
t3yRrWVlUB4t9mD54ur+wPrCw9GXtA==
=vnL1
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/alistair23/tags/pull-riscv-to-apply-20211007' into staging
Third RISC-V PR for QEMU 6.2
- Add Zb[abcs] instruction support
- Remove RVB support
- Bug fix of setting mstatus_hs.[SD|FS] bits
- Mark some UART devices as 'input'
- QOMify PolarFire MMUART
- Fixes for sifive PDMA
- Mark shakti_c as not user creatable
# gpg: Signature made Wed 06 Oct 2021 11:42:53 PM PDT
# gpg: using RSA key F6C4AC46D4934868D3B8CE8F21E10D29DF977054
# gpg: Good signature from "Alistair Francis <alistair@alistair23.me>" [full]
* remotes/alistair23/tags/pull-riscv-to-apply-20211007: (26 commits)
hw/riscv: shakti_c: Mark as not user creatable
hw/dma: sifive_pdma: Don't run DMA when channel is disclaimed
hw/dma: sifive_pdma: Fix Control.claim bit detection
hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
hw/char: sifive_uart: Register device in 'input' category
hw/char: shakti_uart: Register device in 'input' category
hw/char: ibex_uart: Register device in 'input' category
target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
disas/riscv: Add Zb[abcs] instructions
target/riscv: Remove RVB (replaced by Zb[abcs])
target/riscv: Add zext.h instructions to Zbb, removing pack/packu/packh
target/riscv: Add rev8 instruction, removing grev/grevi
target/riscv: Add a REQUIRE_32BIT macro
target/riscv: Add orc.b instruction for Zbb, removing gorc/gorci
target/riscv: Reassign instructions to the Zbb-extension
target/riscv: Add instructions of the Zbc-extension
target/riscv: Reassign instructions to the Zbs-extension
target/riscv: Remove shift-one instructions (proposed Zbo in pre-0.93 draft-B)
...
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Test what happens when there is an I/O error after a mirror job in the
READY phase has been cancelled.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-14-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Clearing .cancelled before leaving the main loop when the job has been
soft-cancelled is no longer necessary since job_is_cancelled() only
returns true for jobs that have been force-cancelled.
Therefore, this only makes a differences in places that call
job_cancel_requested(). In block/mirror.c, this is done only before
.cancelled was cleared.
In job.c, there are two callers:
- job_completed_txn_abort() asserts that .cancelled is true, so keeping
it true will not affect this place.
- job_complete() refuses to let a job complete that has .cancelled set.
It is correct to refuse to let the user invoke job-complete on mirror
jobs that have already been soft-cancelled.
With this change, there are no places that reset .cancelled to false and
so we can be sure that .force_cancel can only be true if .cancelled is
true as well. Assert this in job_is_cancelled().
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-13-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Once the mirror job is force-cancelled (job_is_cancelled() is true), we
should not generate new I/O requests. This applies to active mirroring,
too, so stop it once the job is cancelled.
(We must still forward all I/O requests to the source, though, of
course, but those are not really I/O requests generated by the job, so
this is fine.)
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-12-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
We must check whether the job is force-cancelled early in our main loop,
most importantly before any `continue` statement. For example, we used
to have `continue`s before our current checking location that are
triggered by `mirror_flush()` failing. So, if `mirror_flush()` kept
failing, force-cancelling the job would not terminate it.
Jobs can be cancelled while they yield, and once they are
(force-cancelled), they should not generate new I/O requests.
Therefore, we should put the check after the last yield before
mirror_iteration() is invoked.
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-11-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
mirror_drained_poll() returns true whenever the job is cancelled,
because "we [can] be sure that it won't issue more requests". However,
this is only true for force-cancelled jobs, so use job_is_cancelled().
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-10-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Most callers of job_is_cancelled() actually want to know whether the job
is on its way to immediate termination. For example, we refuse to pause
jobs that are cancelled; but this only makes sense for jobs that are
really actually cancelled.
A mirror job that is cancelled during READY with force=false should
absolutely be allowed to pause. This "cancellation" (which is actually
a kind of completion) may take an indefinite amount of time, and so
should behave like any job during normal operation. For example, with
on-target-error=stop, the job should stop on write errors. (In
contrast, force-cancelled jobs should not get write errors, as they
should just terminate and not do further I/O.)
Therefore, redefine job_is_cancelled() to only return true for jobs that
are force-cancelled (which as of HEAD^ means any job that interprets the
cancellation request as a request for immediate termination), and add
job_cancel_requested() as the general variant, which returns true for
any jobs which have been requested to be cancelled, whether it be
immediately or after an arbitrarily long completion phase.
Finally, here is a justification for how different job_is_cancelled()
invocations are treated by this patch:
- block/mirror.c (mirror_run()):
- The first invocation is a while loop that should loop until the job
has been cancelled or scheduled for completion. What kind of cancel
does not matter, only the fact that the job is supposed to end.
- The second invocation wants to know whether the job has been
soft-cancelled. Calling job_cancel_requested() is a bit too broad,
but if the job were force-cancelled, we should leave the main loop
as soon as possible anyway, so this should not matter here.
- The last two invocations already check force_cancel, so they should
continue to use job_is_cancelled().
- block/backup.c, block/commit.c, block/stream.c, anything in tests/:
These jobs know only force-cancel, so there is no difference between
job_is_cancelled() and job_cancel_requested(). We can continue using
job_is_cancelled().
- job.c:
- job_pause_point(), job_yield(), job_sleep_ns(): Only force-cancelled
jobs should be prevented from being paused. Continue using job_is_cancelled().
- job_update_rc(), job_finalize_single(), job_finish_sync(): These
functions are all called after the job has left its main loop. The
mirror job (the only job that can be soft-cancelled) will clear
.cancelled before leaving the main loop if it has been
soft-cancelled. Therefore, these functions will observe .cancelled
to be true only if the job has been force-cancelled. We can
continue to use job_is_cancelled().
(Furthermore, conceptually, a soft-cancelled mirror job should not
report to have been cancelled. It should report completion (see
also the block-job-cancel QAPI documentation). Therefore, it makes
sense for these functions not to distinguish between a
soft-cancelled mirror job and a job that has completed as normal.)
- job_completed_txn_abort(): All jobs other than @job have been
force-cancelled. job_is_cancelled() must be true for them.
Regarding @job itself: job_completed_txn_abort() is mostly called
when the job's return value is not 0. A soft-cancelled mirror has a
return value of 0, and so will not end up here then.
However, job_cancel() invokes job_completed_txn_abort() if the job
has been deferred to the main loop, which is mostly the case for
completed jobs (which skip the assertion), but not for sure.
To be safe, use job_cancel_requested() in this assertion.
- job_complete(): This is function eventually invoked by the user
(through qmp_block_job_complete() or qmp_job_complete(), or
job_complete_sync(), which comes from qemu-img). The intention here
is to prevent a user from invoking job-complete after the job has
been cancelled. This should also apply to soft cancelling: After a
mirror job has been soft-cancelled, the user should not be able to
decide otherwise and have it complete as normal (i.e. pivoting to
the target).
- job_cancel(): Both functions are equivalent (see comment there), but
we want to use job_is_cancelled(), because this shows that we call
job_completed_txn_abort() only for force-cancelled jobs. (As
explained for job_update_rc(), soft-cancelled jobs should be treated
as if they have completed as normal.)
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-9-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
The only job that supports a soft cancel mode is the mirror job, and in
such a case it resets its .cancelled field before it leaves its .run()
function, so it does not really count as cancelled.
However, it is possible to cancel the job after .run() returns and
before job_exit() (which is run in the main loop) is executed. Then,
.cancelled would still be true and the job would count as cancelled.
This does not seem to be in the interest of the mirror job, so adjust
job_cancel_async() to not set .cancelled in such a case, and
job_cancel() to not invoke job_completed_txn_abort().
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-8-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
We largely have two cancel modes for jobs:
First, there is actual cancelling. The job is terminated as soon as
possible, without trying to reach a consistent result.
Second, we have mirror in the READY state. Technically, the job is not
really cancelled, but it just is a different completion mode. The job
can still run for an indefinite amount of time while it tries to reach a
consistent result.
We want to be able to clearly distinguish which cancel mode a job is in
(when it has been cancelled). We can use Job.force_cancel for this, but
right now it only reflects cancel requests from the user with
force=true, but clearly, jobs that do not even distinguish between
force=false and force=true are effectively always force-cancelled.
So this patch has Job.force_cancel signify whether the job will
terminate as soon as possible (force_cancel=true) or whether it will
effectively remain running despite being "cancelled"
(force_cancel=false).
To this end, we let jobs that provide JobDriver.cancel() tell the
generic job code whether they will terminate as soon as possible or not,
and for jobs that do not provide that method we assume they will.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20211006151940.214590-7-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Callers should be able to specify whether they want job_cancel_sync() to
force-cancel the job or not.
In fact, almost all invocations do not care about consistency of the
result and just want the job to terminate as soon as possible, so they
should pass force=true. The replication block driver is the exception,
specifically the active commit job it runs.
As for job_cancel_sync_all(), all callers want it to force-cancel all
jobs, because that is the point of it: To cancel all remaining jobs as
quickly as possible (generally on process termination). So make it
invoke job_cancel_sync() with force=true.
This changes some iotest outputs, because quitting qemu while a mirror
job is active will now lead to it being cancelled instead of completed,
which is what we want. (Cancelling a READY mirror job with force=false
may take an indefinite amount of time, which we do not want when
quitting. If users want consistent results, they must have all jobs be
done before they quit qemu.)
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-6-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
When a transaction is aborted, no result matters, and so all jobs within
should be force-cancelled.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-5-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
As of HEAD^, there is no meaning to s->synced other than whether the job
is READY or not. job_is_ready() gives us that information, too.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20211006151940.214590-4-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
An error does not take us out of the READY phase, which is what
s->synced signifies. It does of course mean that source and target are
no longer in sync, but that is what s->actively_sync is for -- s->synced
never meant that source and target are in sync, only that they were at
some point (and at that point we transitioned into the READY phase).
The tangible problem is that we transition to READY once we are in sync
and s->synced is false. By resetting s->synced here, we will transition
from READY to READY once the error is resolved (if the job keeps
running), and that transition is not allowed.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20211006151940.214590-3-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Finalizing the job may cause its AioContext to change. This is noted by
job_exit(), which points at job_txn_apply() to take this fact into
account.
However, job_completed() does not necessarily invoke job_txn_apply()
(through job_completed_txn_success()), but potentially also
job_completed_txn_abort(). The latter stores the context in a local
variable, and so always acquires the same context at its end that it has
released in the beginning -- which may be a different context from the
one that job_exit() releases at its end. If it is different, qemu
aborts ("qemu_mutex_unlock_impl: Operation not permitted").
Drop the local @outer_ctx variable from job_completed_txn_abort(), and
instead re-acquire the actual job's context at the end of the function,
so job_exit() will release the same.
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211006151940.214590-2-hreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
