+-- On Thu, 18 Jun 2020, Paolo Bonzini wrote --+
| On 18/06/20 15:12, Alex Bennée wrote:
| > If you look at memory_region_dispatch_write you can see that
| > mr->ops->write being empty is acceptable because it implies
| > mr->ops->write_with_attrs is set instead. I think the same is true for
| > read so I think you need something more like:
| >
| > assert(ops->read || ops->read_with_attrs);
| > assert(ops->write || ops->write_with_attrs);
|
| Also, !ops is acceptable since you have a couple lines below:
|
| mr->ops = ops ? ops : &unassigned_mem_ops;
Oops! :(
| Needless to say, this is something that the submitter should have done,
| not the reviewer.
Sorry, I should've considered full effects of the patch, not just fixing the
NULL dereference issue at hand. Sorry about the haste, I'll be careful in
future.
| > Also doesn't this break a load of running stuff without fixes for all the
| > various missing bits? How far does make check-acceptance get?
I tried to run '$ make check-acceptance', it's breaking with time-out error.
urllib.error.URLError: <urlopen error [Errno 110] Connection timed out>
urllib.error.URLError: <urlopen error [Errno 110] Connection timed out>
...
make: *** [../qemu/tests/Makefile.include:933: get-vm-image-fedora-31-ppc64le]
Error 1
| This might actually be really close with the above assertions fixed.
| For example, commit 08565552f7 ("cputlb: Move NOTDIRTY handling from I/O
| path to TLB path", 2019-09-25) got rid of io_mem_notdirty.
|
| The only cases I found with "git grep" are:
|
| - tz_ppc_dummy_ops which is broken and should just use NULL ops
|
| - hw/nvram/nrf51_nvm.c's flash_ops which is fixed if ROMD regions are
| changed not to require a read callback.
|
| - designware_pci_host_msi_ops which is broken and should have a dummy
| read callback.
ie. we add these routines along with the assertions here, right?
Earlier patch series adds routines for nrf51_nvm and designware. I'll add
r/w routines for tz_ppc_dummy_ops.
Thank you so much for the review.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
