On Mon, 17 Oct 2022 at 14:50, Alexander Bulekov <alx...@bu.edu> wrote: > > On 221015 1710, Chris Friedt wrote: > > From: Christopher Friedt <cfri...@meta.com> > > > > In the case that size1 was zero, because of the explicit > > 'end1 > addr' check, the range check would fail and the error > > message would read as shown below. The correct comparison > > is 'end1 >= addr' (or 'addr <= end1'). > > > > EDU: DMA range 0x40000-0x3ffff out of bounds (0x40000-0x40fff)! > > > > At the opposite end, in the case that size1 was 4096, within() > > would fail because of the non-inclusive check 'end1 < end2', > > which should have been 'end1 <= end2'. The error message would > > previously say > > > > EDU: DMA range 0x40000-0x40fff out of bounds (0x40000-0x40fff)! > > > > This change > > 1. renames local variables to be more less ambiguous > > 2. fixes the two off-by-one errors described above. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1254 > > > > Signed-off-by: Christopher Friedt <cfri...@meta.com> > > Reviewed-by: Alexander Bulekov <alx...@bu.edu> > > As a side-note, seems strange that edu_check_range will abort the entire > VM if the check fails, rather than handling the error more elegantly.
Yes, this is bad for a device model, though we have a lot of older device models that still do it. The preferred pattern is: * for situations which are "if this happens there is a bug in QEMU itself", use assert. hw_error() is a kind of assert that prints a bunch of guest register state: sometimes you want that, but more often you just want normal assert() * for situations where the guest has misprogrammed the device, log that with qemu_log_mask(LOG_GUEST_ERROR, ...) and continue with whatever the real hardware would do, or some reasonable choice if the h/w spec is vague * for situations where the guest is correct but is trying to get the device to do something our model doesn't implement yet, same as above but with LOG_UNIMP. Probably most hw_error() uses in the codebase should be replaced with one of the above options. thanks -- PMM
