On 11.05.2018 11:11, Paolo Bonzini wrote: > On 07/05/2018 21:40, David Hildenbrand wrote: >> On 07.05.2018 18:19, Paolo Bonzini wrote: >>> On 07/05/2018 11:22, David Hildenbrand wrote: >>>>> >>>>> +/* Bits which may be returned by set_spte() */ >>>>> +#define WRPROT_SHADOW_PT BIT(0) >>>>> +#define NEED_FLUSH_REMOTE_TLBS BIT(1) >>>> Not sure if I really like returning flags. Especially as the naming does >>>> not suggest that or that these values are somehow linked together. >>>> >>>> What about a flag &flush? >>>> >>> >>> I think the flags are okay (I dislike bool return values in general), >>> but the naming can be improved, for example SET_SPTE_WRITE_PROTECTED and >>> SET_SPTE_NEED_REMOTE_FLUSH. >> >> The thing I don't like about flags is that you cannot properly handle >> error scenarios anymore (return -EWHATEVER). But as there are no errors >> scenarios to handle, fine with me :) > > If the errors are negative and the flags are <64 they can be handled > too, can't they? I see what you mean. e.g. -1 -> 0xfffff.. -> all flags set. But if the caller always checks for <0 it's not a problem. So it's all about the check for bit 64 in the caller a.k.a < 0 > > Paolo > -- Thanks, David / dhildenb