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

Re: inconsistent pfn type checking in process_page_data

  • To: Olaf Hering <olaf@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Tue, 27 Oct 2020 23:18:47 +0000
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Delivery-date: Tue, 27 Oct 2020 23:19:06 +0000
  • Ironport-sdr: VnByQLNSlIX7iD4ifp24aPZTiaBZXb++wlCSUecghkT5Hdd25WUekYvZve6UplKRQleaiTQtgd WkrGun2nbcO3AwraMFnC1X30E7J8x7f5RaJKoTgfdSlmkQpmbgSBNscY8eIDGePLotzuK//ry1 L5DelmyXWZixd3tRpj0RZ/+4uARrbr8l6gCwI+f4O21o0HdFrv+CYyVGIA+S+e9u4+C/wFuOAJ Hd1YCS/ZbBPfGVL/e8pa9DFPAsbsfR3bylLRs+4CHRGfc+XDkoDzeBpAK/QrGkozEApoZDMhYz p/w=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 27/10/2020 17:41, Olaf Hering wrote:
> Andrew,
> with commit 93c2ff78adcadbe0f8bda57eeb30b1414c966324 a new function 
> process_page_data was added. While filling the mfns array for 
> xenforeignmemory_map, the individual pfn types[] are checked in a different 
> way than the checking of the result of the mapping attempt. 
> Is there a special reason why the first loop uses the various TAB values, and 
> the other loop checks XTAB/BROKEN/XALLOC? The sending side also uses the 
> latter.

Hmm.  (Wow that's more than 5 years old now)

IIRC, that particular piece of logic took an age to get correct.  Its
quite possible it ended up being a merge of code which David and I wrote
at slightly different times.

Fundamentally, there are 4 bits of page type which encode NOTAB
(writeable), L{1..4}, Pinned L{1..4}, XTAB (invalid), XALLOC
(allocate-only - still not sure what the purpose is.  There is no
producer of this type that I'm aware of) and BROKEN.

By my count, that is 12 legal types, so asymmetric type=>"is there data"
conversions is probably a bad thing.

I suspect we probably want an is_known_page_type() predicate on the
source side to sanity check data from Xen, and on the destination side
in handle_page_data() to sanity check data in the stream, and we
probably want a page_type_has_data() predicate to use in
write_batch()/process_page_data() to ensure that the handling is consistent.

Do you fancy doing a patch?  If not I can put this on my TODO list, but
no ETA on when I'd get to it.


Attachment: signature.asc
Description: OpenPGP digital signature



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